Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Messages][JavaScript] Add schemas to npm package #2010

Merged
merged 4 commits into from
Jun 20, 2022

Conversation

aurelien-reeves
Copy link
Contributor

Summary

The dedicated repo for the CCK requires access to messages json schemas. An easy way to do so is to publish the schemas with the messages NPM package.

Actually that may be a nice addition to the package itself even beside our own primary need. We may later consider publishing those schemas with other implementations of messages.

Details

We copy the schemas as part of the .codegen make target

@mattwynne
Copy link
Member

To release this, we still need to modify package.json to include these files in the published package.

@aurelien-reeves aurelien-reeves enabled auto-merge (squash) June 20, 2022 14:11
@aurelien-reeves aurelien-reeves merged commit ff6e215 into main Jun 20, 2022
@aurelien-reeves aurelien-reeves deleted the add-schemas-to-js-package branch June 20, 2022 14:57
@mattwynne
Copy link
Member

This is released in messages/v19.1.0

@aurelien-reeves
Copy link
Contributor Author

To release this, we still need to modify package.json to include these files in the published package.

We actually did not do that part 😶

@ciaranmcnulty
Copy link
Contributor

Late to this but FWIW why not have the schemas in their own subrepo with their own package.json? That way if I wanted them in PHP I could add a composer.json... etc

@aurelien-reeves
Copy link
Contributor Author

aurelien-reeves commented Jun 21, 2022

@ciaranmcnulty that was on the table indeed (refs. cucumber/compatibility-kit#5)

We've picked the option to embed those as part of the @cucumber/messages npm package in order to move fast with this and to comply with the original need easily and quickly. We need this at first to move-on with moving-out the CCK from the monorepo.

Yet we agree that:

  • having those schemas as part of all messages implementation
  • or having dedicated packages for the schema in different implementations

could be nice things to have "later".

And as I write that comment, I realize that those schemas could actually also be part of the CCK 🤔 .

Do you know that we have a CCK for javascript and for ruby, and that we could also have a CCK for php, or any other languages? ☺️

@mattwynne
Copy link
Member

To release this, we still need to modify package.json to include these files in the published package.

We actually did not do that part 😶

Gah! So we need a 19.1.1 release to fix that?

@mattwynne
Copy link
Member

And as I write that comment, I realize that those schemas could actually also be part of the CCK 🤔 .

As I see it, there are three levels of abstraction that we could express in the dependencies:

  1. The schemas themselves - an abstract representation of the data structures we use for the message protocol
  2. The message DTOs - packages that provide concrete implementations of those data structures in a given programming language
  3. The CCK - a tool that uses those DTOs and schemas to validate the behaviour of Cucumber implementations against our expectations of the message protocol.

I see the CCK as something analogous to a contract test for the message protocol in that it validates an implementation of the message protocol not just in terms of message structure, but message content and sequence, in different scenarios.

Whether we need to separate the schemas out to another package now or in the long term to better represent these abstractions, I'm not sure.

@aurelien-reeves
Copy link
Contributor Author

To release this, we still need to modify package.json to include these files in the published package.

We actually did not do that part 😶

Gah! So we need a 19.1.1 release to fix that?

Fixed in 19.1.2

19.1.1 did not worked as expected either

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants