-
-
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
Emulate Cucumber-JS's new i18n CLI options #1140
Emulate Cucumber-JS's new i18n CLI options #1140
Conversation
@aidamanna May I suggest that you edit the pull-request description to include a keyword for closing issues, to automatically close #1137 when this pull-request is merged. For instance changing " |
lib/cucumber/cli/options.rb
Outdated
elsif !::Gherkin::DIALECTS.keys.include? lang | ||
indicate_invalid_language_and_exit(lang) | ||
else | ||
if ::Gherkin::DIALECTS.keys.include? lang |
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.
Now that this method is shorter, could you swap this logic around round to use a Guard Clause?
i.e.
return indicate_invalid_language_and_exit(lang) if !::Gherkin::DIALECTS.keys.include? lang
list_keywords_and_exit(lang)
@aidamanna this is lovely, a great first contribution! I see that GitHub is warning us there are merge conflicts, due to you and I both fixing the rubocop offences I left behind in Can you try rebasing this branch from master and then force pushing it? Give me a shout in Slack if you need a hand doing that. |
Also, I think if you rebase master the build may go green - I've tagged out the scenario that seems to be flickering / timing-out on JRuby. |
@mattwynne little refactor done! :) |
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. |
Summary
Emulates Cucumber-JS's new i18n CLI options.
Fixes #1137 and fixes a test.
Details
See issue #1137
Motivation and Context
Keeping things consistent between implementations.
How Has This Been Tested?
Existing tests have been modified to meet the new requirements.
Types of changes
Checklist:
Please review @mattwynne @tooky