-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[Core] Refactor CommandLine arguments #2130
Conversation
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.
This is useful. Since this class is to be used in combination with the main class it would make sense to put it in the same package. It is essentially an interface for public consumption..
core/src/main/java/io/cucumber/core/options/CommandlineOptions.java
Outdated
Show resolved
Hide resolved
- Rename for shorter naming - Move to io.cucumber.core.cli - Annote with @API - Make the class final, disable instantiating
core/src/main/java/io/cucumber/core/options/CommandlineOptionsParser.java
Outdated
Show resolved
Hide resolved
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.
Almost there!
core/src/main/java/io/cucumber/core/cli/CommandlineOptions.java
Outdated
Show resolved
Hide resolved
I pushed some changes. Let me know what you you think. |
Thanks @mpkorstanje , everything seems to be great now. But for later PRs, what changes will be categorized as |
We're using Keep a Changelog as a guide: https://keepachangelog.com/en/1.0.0/
Since this wasn't existing behaviour, changed does not apply. |
@mpkorstanje Got it. Everything is clear by now, please continue PR process as I don't have any further comments. Thank you! |
Hi @ltpquang, 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, |
Define CLI arguments as constants instead of hardcoded strings. Co-authored-by: M.P. Korstanje <rien.korstanje@gmail.com>
Problem
I'm about to call Cucumber
main
function from my own appFor better management, I would standardize my app's arguments, before passing to Cucumber.
The problem is Cucumber's arguments are now hardcoded, which make future updates maybe break my app.
Solution
Define all arguments as static string variables. This makes arguments more consistent, and cleaner codebase.