Conversation
There was a problem hiding this comment.
Pull request overview
This PR is a v17-focused refactor/cleanup aimed at reducing redundant requires, tightening APIs (notably requiring an event bus), and doing small internal reorganizations.
Changes:
- Remove
cucumber/core/platformand references to it. - Switch several internal
requirestatements torequire_relativeand reorder methods for readability. - Make
Cucumber::Core::Compilerrequire a non-nilevent_busand always emit creation events.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/cucumber/core/test/case_spec.rb | Removes unused platform require. |
| spec/cucumber/core_spec.rb | Removes unused platform require. |
| lib/cucumber/core/test/timer.rb | Uses require_relative for local dependency loading. |
| lib/cucumber/core/test/tag.rb | Adds explicit local dependency (location). |
| lib/cucumber/core/test/step.rb | Converts to require_relative and reorders methods. |
| lib/cucumber/core/test/location.rb | Removes unused dependencies (forwardable, platform). |
| lib/cucumber/core/test/hook_step.rb | Uses require_relative. |
| lib/cucumber/core/test/filters.rb | Uses require_relative for filters. |
| lib/cucumber/core/test/empty_multiline_argument.rb | Reorders methods (no functional change intended). |
| lib/cucumber/core/test/doc_string.rb | Minor formatting and method reordering. |
| lib/cucumber/core/test/data_table.rb | Refactors method order / docs; adds inspect/lines_count placement changes. |
| lib/cucumber/core/test/case.rb | Uses require_relative and reorders methods. |
| lib/cucumber/core/test/around_hook.rb | Reorders hook? definition. |
| lib/cucumber/core/test/action.rb | Converts to require_relative for local dependencies. |
| lib/cucumber/core/platform.rb | Deletes platform detection module. |
| lib/cucumber/core/gherkin/parser.rb | Simplifies message type selection logic. |
| lib/cucumber/core/gherkin/document.rb | Changes defaulting behavior for language. |
| lib/cucumber/core/compiler.rb | Makes event_bus mandatory; removes safe-navigation when emitting events. |
| .rubocop_todo.yml | Regenerates todo config header/entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def transpose | ||
| self.class.new(raw.transpose) | ||
| def inspect | ||
| %{#<#{self.class} #{raw.inspect})>} |
There was a problem hiding this comment.
inspect returns a string with an extra ) before >, which makes the output malformed (e.g. #<... ...)>). Remove the stray parenthesis so the inspected representation is well-formed.
| %{#<#{self.class} #{raw.inspect})>} | |
| %{#<#{self.class} #{raw.inspect}>} |
| # Creates a new instance. +raw+ should be a square (2d), array of strings or an array of hashes | ||
| # | ||
| # You don't typically create your own DataTable objects - Cucumber will do | ||
| # it internally and pass them to your Step Definitions. | ||
| # | ||
| def initialize(rows) | ||
| raw = ensure_array_of_array(rows) |
There was a problem hiding this comment.
The initializer doc comment says +raw+ but the parameter is rows. Update the comment to match the actual argument name (or rename the parameter) to avoid confusing API consumers.
| def initialize(uri, body, language = 'en') | ||
| @uri = uri | ||
| @body = body | ||
| @language = language |
There was a problem hiding this comment.
Changing the signature to language = 'en' subtly changes behavior when callers pass nil explicitly: it will now set @language to nil instead of defaulting to 'en'. If the intention is to keep the previous behavior, coerce nil to 'en' (e.g. @language = language || 'en').
| @language = language | |
| @language = language || 'en' |
Description
Fixes a few minor / not important things in terms of code style/optimisation.
All pending to be completed for a release of v17
Type of change
work as expected)
Please add an entry to the relevant section of CHANGELOG.md as part of this pull request.
Note to other contributors
This might be considered a breaking change as I don't permit not supplying an event bus (But this shouldn't have ever been done by anyone)
The Cucumber::Platform file contained a bunch of constants that weren't ever able to be set because we've had a Cucumber::VERSION for a long time now
Checklist:
Your PR is ready for review once the following checklist is
complete. You can also add some checks if you want to.
bundle exec rubocopreports no offenses