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

Json schema for message protocol #1414

Merged
merged 80 commits into from
Apr 29, 2021
Merged

Json schema for message protocol #1414

merged 80 commits into from
Apr 29, 2021

Conversation

aslakhellesoy
Copy link
Contributor

@aslakhellesoy aslakhellesoy commented Mar 10, 2021

This is a Draft PR to replace protobuf with pure JSON and JSON Schema, addressing #1386.

TODO:

  • TypeScript: Modify ts code generator to define a Status alternate type from the enum in the schema. Same for ContentEncoding
  • Working PR for cucumber-js
  • Working PR for cucumber-ruby
  • Working PR for cucumber-jvm

Later:

  • Validate all generated messages in compatibility kit with the JSON schema.
  • Elixir implementation/code generator
  • .NET implementation/code generator
  • Go implementation/code generator

We can live with a combination of protobuf/json schema implementations for some time, as long as we keep them in sync. The gherkin acceptance tests will catch any inconsistencies.

@WannesFransen1994
Copy link
Contributor

Not sure if it's relevant, but following libraries might be helpful for the Elixir code generation:

@aslakhellesoy
Copy link
Contributor Author

Thanks @WannesFransen1994 we'll have a look at those.

@aslakhellesoy aslakhellesoy marked this pull request as ready for review April 28, 2021 23:25
@aurelien-reeves
Copy link
Contributor

The removal of "sudo" before installing npm globally was the thing to fix the CI? 😨

This is a big PR to review. Here's a big list of question 😅

  • What should we focus on while reviewing?
  • What are the next steps?
  • When could it be merged?
  • Should we wait for the PR in cucumber-js, cucumber-ruby and cucumber-jvm to be ready too?
  • Should we wait for .Net and Elixir to be implemented?

@aslakhellesoy
Copy link
Contributor Author

aslakhellesoy commented Apr 29, 2021

The removal of "sudo" before installing npm globally was the thing to fix the CI? 😨

This is a big PR to review. Here's a big list of question 😅

  • What should we focus on while reviewing?

Primarily finding breaking changes in APIs and data structures and update changelogs

  • What are the next steps?

Merge to master and continue working on Cucumber PRs

  • When could it be merged?

I think we can merge now, unless there are objections.

  • Should we wait for the PR in cucumber-js, cucumber-ruby and cucumber-jvm to be ready too?

I think we can work on those after this is merged.

  • Should we wait for .Net and Elixir to be implemented?

And Perl. I think we should disable them all (they already are). Waiting for them will cause too much delay. They can catch up at their own pace. We should create an issue for all 3 of them.

@aslakhellesoy aslakhellesoy merged commit 0099611 into master Apr 29, 2021
Cucumber Open automation moved this from In Progress to Implemented Apr 29, 2021
@aslakhellesoy aslakhellesoy deleted the json-schema branch April 29, 2021 14:36
@mattwynne mattwynne moved this from Implemented to Released in Cucumber Open May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

7 participants