Upgrade messages and gherkin to bring new 'jsonschema messages'

[aurelien-reeves]

No description provided.

[mattwynne]

@aslakhellesoy I hadn't noticed that the change to the messages library had changed to exposing the messages as hashes instead of objects. It strikes me that it might be better to use wrap the bare message hashes in POROs, to give us an isolation layer so we don't need to make sweeping changes like this. Classes would also make it easier to document the messages with RDoc, and make the message protocol a bit more discoverable - hashes don't give good error messages if you typo a field name.

It also might help prevent confusion like cucumber/common#1559.

What do you think? Was the decision to use hashes over objects just a pragmatic one to get something released, or was there some more intent behind it?

[aslakhellesoy]

I like that suggestion @mattwynne. To make it a reality, someone would have to build a code generator for ruby, similar to the one we have for go and typescript.

The reason I went with hashes? It didn’t occur to me that we’d need classes. My main motivation for doing code generation have been:

• TypeScript: type safety and default values in deserialisation
• Go: Required for JSON output

[aslakhellesoy]

There is additional complexity with classes. If you want to maintain backwards compatibility with the Ruby API, all attributes would have to be snake_case, but the JSON representation needs to be camelCase.

I’m not convinced adding classes is worth it for ruby messages, because there are so few libs depending on it (AFAIK).

[aurelien-reeves]

I would be in favor of DTOs to bring a more consistent public API over time, and to be more ruby-friendly too.

Also, messages are consumed directly by formatters, so they may be used more than we could think of as I think custom formatters may be the first way to extend cucumber?

[aslakhellesoy]

Fair enough. The starting point would be to add a code generator for those DTOs as done here:

• https://github.com/cucumber/common/blob/main/messages/go/Makefile
• https://github.com/cucumber/common/blob/main/messages/javascript/Makefile
• https://github.com/cucumber/common/blob/8abf7188dd83e462f7944fdb1be70a1c5ce54455/messages/jsonschema/scripts/codegen.rb#L157
• https://github.com/cucumber/common/blob/8abf7188dd83e462f7944fdb1be70a1c5ce54455/messages/jsonschema/scripts/codegen.rb#L107

Then update code to use those DTO classes:

• https://github.com/cucumber/common/blob/main/messages/ruby/spec/cucumber/messages/ndjson_serialization_spec.rb
• https://github.com/cucumber/common/tree/main/gherkin/ruby/lib/gherkin
• https://github.com/cucumber/common/tree/main/demo-formatter/ruby

[mattwynne]

I've created a ticket in the common repo about this: cucumber/common#1574

[luke-hill]

@aurelien-reeves this might be a blocker on getting my PR complete (As I would need a patched version of messages to the new 19.0.3 (Not yet created). Which seems from this PR to require lots of changes.

[aurelien-reeves]

@aurelien-reeves this might be a blocker on getting my PR complete (As I would need a patched version of messages to the new 19.0.3 (Not yet created). Which seems from this PR to require lots of changes.

In conjuction with cucumber-ruby-core and cucumber-ruby-wire, this PR is actually ready. If you execute the tests using cucumber/cucumber-ruby-core#219, they are passing.

So the question is:

1. do we agree with releasing a cucumber-ruby with messages represented as plain ruby hashes with camelCase keys to release the fixes for rules asap?
2. do we wait for messages with DTOs instead of hashes and delay fixes for rules for some time?
3. do we fix Gherkin@18.1 which rely on messages@15 to release the fix for rules more quickly, but make us deploy a fix for a past version which we are not use to?

I must say that I don't really have a favorite. If we go for 2. waiting for messages to have a release with DTOs, I can organize myself to be super-focus on that next week to try releasing it as quickly as possible.

[luke-hill]

My vote is for number1. But at the same time I'm now going to be out of action until basically next weeks meeting. So I don't necessarily want to push urgently for something.

At the same time it would be good to think about a timeline for getting my PR merged (As it enables a reasonable chunk of functionality, even though it's small). So ideally some time before end of june would be preferred.

[mattwynne]

If we can get the DTOs done, most of the changes in this PR would not be necessary. I think it's probably better to delay merging this change until we have the DTOs done, to minimise churn in the code.

[aurelien-reeves]

Once cucumber/cucumber-ruby-wire#44 is merged, we can come back to the use of cucumber-ruby-wire main branch.

[aurelien-reeves]

I have to merge this in order to be able to release wire because wire depends on the main github branch.

But I won't release cucumber-ruby without proper review