-
-
Notifications
You must be signed in to change notification settings - Fork 695
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
PHP Gherkin Parser: v24 is not compatible with v18 of messages #2034
Conversation
Thanks @dantleech that's annoying What would be the best away to detect this? Add |
yes, that's what I'm doing with GH actions |
Thanks @dantleech for the patch 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An entry in the changelog is missing actually
Do we try to add a non-regression test as part of this PR? Or do we quickly release the patch asap and work on a new test in another PR? |
I think it should be merged with changelog entry I am so confused by the Makefiles in this repo that I'm to sure how to add a second job effectively :/ It basically needs a second CI job that can run |
I'll take a look as soon as I have a moment 👌 |
@aurelien-reeves we could defer until repo split puts it all in GHA ;) |
This is mostly because of #2034 - we need to check that everything still works with the 'lowest' versions of the dependencies There aren't any external dependencies yet so currently this will only detect when Gherkin doesn't work with Messages any more, or when we've written tests that don't work with older versions of the test tools
This is mostly because of #2034 - we need to check that everything still works with the 'lowest' versions of the dependencies There aren't any external dependencies yet so currently this will only detect when Gherkin doesn't work with Messages any more, or when we've written tests that don't work with older versions of the test tools
@aurelien-reeves actually by explaining it I realised maybe it's easy #2035 |
This is mostly because of #2034 - we need to check that everything still works with the 'lowest' versions of the dependencies There aren't any external dependencies yet so currently this will only detect when Gherkin doesn't work with Messages any more, or when we've written tests that don't work with older versions of the test tools
This is mostly because of #2034 - we need to check that everything still works with the 'lowest' versions of the dependencies There aren't any external dependencies yet so currently this will only detect when Gherkin doesn't work with Messages any more, or when we've written tests that don't work with older versions of the test tools
This is mostly because of #2034 - we need to check that everything still works with the 'lowest' versions of the dependencies There aren't any external dependencies yet so currently this will only detect when Gherkin doesn't work with Messages any more, or when we've written tests that don't work with older versions of the test tools
Before merging this I'd like to have a small stab at making it backwardly compatible |
Oh wait the testdata has the new fields in, no chance :) |
I would have been pleased to find some ways to make it backward compatible indeed. Having distinct repos in a near future will certainly help improving backward compatibility when working on new features like we did |
Hi @dantleech, Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾 In return for this generous offer we hope you will:
On behalf of the Cucumber core team, |
This is mostly because of #2034 - we need to check that everything still works with the 'lowest' versions of the dependencies There aren't any external dependencies yet so currently this will only detect when Gherkin doesn't work with Messages any more, or when we've written tests that don't work with older versions of the test tools
* Check lowest versions of dependencies This is mostly because of #2034 - we need to check that everything still works with the 'lowest' versions of the dependencies There aren't any external dependencies yet so currently this will only detect when Gherkin doesn't work with Messages any more, or when we've written tests that don't work with older versions of the test tools * Fix COMPOSER_FLAGS var in Makefile * Switch install to update as we gitignore composer.lock * Attempt to fix php 'bc' builds * Update ramsey/uuid to minimum 4.2.1 The following fix is needed: ramsey/uuid#383 * Explicitly depend on high version of nikic/php-parser This is used in static analysis and older versions can't deal with enums directly Becuase we don't depend on it directly Psalm was allowing an older version to install * Silence a static analysis error in tests This is something we still want to check for even if the types say it's redundant - JSON decoding could be throwing an error here * Remove path repository for low-dependency build Co-authored-by: aurelien-reeves <[email protected]>
* Check lowest versions of dependencies This is mostly because of cucumber/common#2034 - we need to check that everything still works with the 'lowest' versions of the dependencies There aren't any external dependencies yet so currently this will only detect when Gherkin doesn't work with Messages any more, or when we've written tests that don't work with older versions of the test tools * Fix COMPOSER_FLAGS var in Makefile * Switch install to update as we gitignore composer.lock * Attempt to fix php 'bc' builds * Update ramsey/uuid to minimum 4.2.1 The following fix is needed: ramsey/uuid#383 * Explicitly depend on high version of nikic/php-parser This is used in static analysis and older versions can't deal with enums directly Becuase we don't depend on it directly Psalm was allowing an older version to install * Silence a static analysis error in tests This is something we still want to check for even if the types say it's redundant - JSON decoding could be throwing an error here * Remove path repository for low-dependency build Co-authored-by: aurelien-reeves <[email protected]>
* Check lowest versions of dependencies This is mostly because of cucumber/common#2034 - we need to check that everything still works with the 'lowest' versions of the dependencies There aren't any external dependencies yet so currently this will only detect when Gherkin doesn't work with Messages any more, or when we've written tests that don't work with older versions of the test tools * Fix COMPOSER_FLAGS var in Makefile * Switch install to update as we gitignore composer.lock * Attempt to fix php 'bc' builds * Update ramsey/uuid to minimum 4.2.1 The following fix is needed: ramsey/uuid#383 * Explicitly depend on high version of nikic/php-parser This is used in static analysis and older versions can't deal with enums directly Becuase we don't depend on it directly Psalm was allowing an older version to install * Silence a static analysis error in tests This is something we still want to check for even if the types say it's redundant - JSON decoding could be throwing an error here * Remove path repository for low-dependency build Co-authored-by: aurelien-reeves <[email protected]>
Summary
Updated
composer.json
to require at least v19 of cubumber PHP messagesDetails
When using v24 with lowest dependencies:
We get the following error:
With v19.0.0 of cucumber/messages it works.
Types of changes