-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Integrate new core events #973
Conversation
@@ -21,7 +21,7 @@ Thanks! | |||
♡ The Cucumber Team ♡ | |||
} | |||
|
|||
s.add_dependency 'cucumber-core', '~> 1.4.0' | |||
s.add_dependency 'cucumber-core', '~> 3.0' |
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.
The cucumber-core
version has been changed to "2.0"
Now when #920 has been merged, the file |
|
||
# @private | ||
def_instance_delegator :event_bus, :notify | ||
def notify(message, *args) | ||
event_bus.send(message, *args) |
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.
The event bus does also have a broadcast
method that takes an event class instance as argument. Should it also be made available through the configuration object?
@io = ensure_io(config.out_stream) | ||
@feature_hashes = [] | ||
config.on_event :test_case_starting, &method(:on_before_test_case) |
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.
I think it would be better to name the methods call on an event with the pattern: on_<event_name>
Thanks for merging @brasmusson. Brave move! I think this constitutes a breaking API change - the events API has changed, so we should release it as 3.0. Shall we have the master branch target 3.0 now? I had hoped to get all the formatters replaced and deprecate the old formatter API with a 3.0 release, but we could do that in a 4.0 release. |
If this should be released as 3.0, then we should have the master branch targeting 3.0. As there are a few bugfixes and upgrade to Gherkin 4.0 unreleased on the master before this, we should branch or maintenance branches before this merge and release a cucumber-core v1.5.0 gem and a cucumber v2.4.0 gem, I think (and then not intend to do anything else on those maintenance branches, but aiming that v3.0 will be the next release). |
Yes, I agree @brasmusson. |
If anyone has time, tidying up the milestones (and assignment of issues to them) to reflect this decision would be a useful exercise. |
Now are 'cucumber' v2.4.0 and 'cucumber-core' v1.5.0 released (from the new branches v2.x-bugfix and v1.x-bugfix. |
I should be able to do that @mattwynne |
Fantastic, thanks @brasmusson! Protocol-wise, do you think we should blog an announcement (e.g. https://cucumber.io/blog/2015/09/11/cucumber-2.1) and link to that from the google group, or just post on the google group? I've not been very consistent in the past - it would be good to just make a rule and stick with it. |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
I decided to move the four main events (test case / step starting / finished) down to the core, so that we don't need a special "report" API anymore. The EventBus has moved down to the core too. I extended it a bit so you can give it multiple namespaces for events, so we can use a mix of
Cucumber::Core::Events
andCucumber::Events
.Motivation and Context
This simplifies the code, and allows more freedom / flexibility for subscribing to events from the runner.
Type of change
This is a breaking change to the events API, and will need to go into a major release (slated for 3.0)
TODO: