[Upgrade Tool] Chores / Quality of Life Improvements #19053
Conversation
This option will allow automatically answering yes to every CLI prompts
… upgrade-tool/chores
| @@ -15,6 +15,11 @@ const addReleaseUpgradeCommand = (releaseType: Version.ReleaseType, description: | |||
| .option('-n, --dry', 'Simulate the upgrade without updating any files', false) | |||
| .option('-d, --debug', 'Get more logs in debug mode', false) | |||
| .option('-s, --silent', "Don't log anything", false) | |||
| .option( | |||
| '-y, --yes', | |||
There was a problem hiding this comment.
We use "force" everywhere else, like data-transfer. The only reason I would switch to "yes" is if we consider force="yes to all", yes="yes to things that are non-destructive" (and thus have both available here)
There was a problem hiding this comment.
For me, it doesn't mean the same thing. Force means "go for it even if we have weird stuff on the way" whereas "yes" means... answering "yes" to all prompts where you can answer "yes". I don't know if that makes sense though
There was a problem hiding this comment.
Although for now, I agree it is similar, since the only things we're answering to are optional requirements
There was a problem hiding this comment.
Yeah, it would technically be 'yes' in this case and then we would introduce 'force' if we encountered something like "run this codemod even if it's unpredictable based on your code" but my worry is that we already use force for data-transfer and now we'd have two different wordings that don't really make that distinction clear.
@marcoautiero your thoughts?
There was a problem hiding this comment.
but my worry is that we already use force for data-transfer and now we'd have two different wordings that don't really make that distinction clear.
I agree, it was my worry too. But I believe that in the long run having well-documented atomic options/flags, is more helpful than trying to accommodate everything under a big one-do-it-all
Also, yes is quite a well-known flag since it's already used quite a lot by package managers & other CLI. So I don't think people would be lost
There was a problem hiding this comment.
We use force to ignore prompts with experimental commands in the strapi CLI by the way. I originally wrote yes but Ben highlighted that we use force in DTS.
There was a problem hiding this comment.
Well, we do use force in the DTS, but again, I feel like it's not aiming at doing the same thing 😕
I don't really see why both couldn't live together
There was a problem hiding this comment.
I don't have a problem with yes, but imo if you are ignoring prompts with yes i think you need to change the CLI too because that is doing the same thing as here, otherwise it's just an inconsistency.
There was a problem hiding this comment.
Do we have potentially destructive effects here?
There was a problem hiding this comment.
Not in the current prompts
packages/utils/upgrade/src/tasks/upgrade/requirements/common.ts
Outdated
Show resolved
Hide resolved
|
Also, looks like unit tests are failing since merging the |
I mean... What?
|
|
Removing the |
|
Just to clarify the test failing come from this PR: #19009 |
You're completely right, I even commented it, my bad 🤦♂️ |
|
Fixed the issue in 1f39b5b The mock used in the test file made The issue appeared because of |
|
Size Change: 0 B Total Size: 627 kB ℹ️ View Unchanged
|
What does it do?
-y, --yesCLI option formajor,minor, andpatchcommands (cc @pwizla)false, automatically answersyesto every prompt (for now, it'll basically ignore optional requirements and proceed anyway)majorupgrade requirements required (were optional up until now)Why is it needed?
Quality of life improvements
How to test it?
yesshould ignore prompts & answer yes anyway (easy to test with the GIT_CLEAN requirement, the warning should be prompted but the process should continue)upgrade majorprocess should fail (as there are no V5 to upgrade to anyway)