Rename --tests option to --skip-tests in Quarkus CLI
#18438
Conversation
|
Thanks for your pull request! The title of your pull request does not follow our editorial rules. Could you have a look?
|
quarkus-bot
bot
added
area/cli
|
@gsmet Adding you as a review since this is potentially a breaking change, but in reality, I don't think it will affect any users since the current |
tqvarnst
changed the title
--tests to --skip-tests, fixes #18391--tests to --skip-tests
geoand
changed the title
--tests to --skip-tests--tests option to --skip-tests in Quarkus CLI
| @@ -13,7 +13,7 @@ | |||
| @CommandLine.Option(order = 5, names = { "--offline" }, description = "Work offline.", defaultValue = "false") | |||
| public boolean offline = false; | |||
|
|
|||
| @CommandLine.Option(order = 6, names = { "--tests" }, description = "Run tests.", negatable = true) | |||
| @CommandLine.Option(order = 6, names = { "--skip-tests" }, description = "Skip tests.") | |||
| public boolean runTests = true; | |||
There was a problem hiding this comment.
Shouldn't the meaning of this property be changed too? I suspect it won't work as is (or I might miss how Picocli is working as I have no experience with it).
There was a problem hiding this comment.
I don't agree with this change. I am on PTO, but I would prefer to make the options as they exist work (which I thought I had tests for, but apparently not...).
|
This will require a proper rebase to avoid the merge commit before merging. |
|
i don't agree with this. I would like to fix --tests and --no-tests. |
|
I'm closing this one as apparently this is not what we want. @ebullient can you take care of the follow-up? Thanks! |
|
back today.. yes, I will |
Fix #18391
Currently, the CLI is documented to default run the test and then the user can control it with --tests or --no-tests. However, the way that Piccoli works a boolean command line option works adding the option will flip the default value, which means that when you add --tests it actually set io.quarkus.cli.common.BuildOptions.runTest to false, and the negotiated flag --no-tests set it to true given the inverted behavior of what was intended.
In this pull request I suggest that we rename the parameter to
--skip-testsremove the negotable option, which gives us the intended behavior where default the CLI build task will run tests, but you can turn if off with--skip-tests.