feat: Add overwrite option to download command #785
Conversation
jonassvalin
changed the title
| @@ -25,21 +26,24 @@ const options = { | |||
| exclude: ['parsed', 'manifest.json', ...extraExclude], | |||
| apiHost: cliOptions.host, | |||
| channelId: cliOptions.channel_id, | |||
| overwrite: cliOptions.overwrite, | |||
There was a problem hiding this comment.
If this is just a boolean, I would prefer more specific name, like skipOverwriteQuestion or omitOverwriteAsk.
There was a problem hiding this comment.
So this one isn't actually saying whether to ask the question, but is the actual answer to the question: Should we overwrite? Hence why it seems correct for it to just be called overwrite
There was a problem hiding this comment.
I have refactored this to simply be a boolean, rather than a "Yes" or "No" string value
bin/stencil-download.js
Outdated
| ? [opts.overwrite] | ||
| : await inquirer.prompt([ | ||
| { | ||
| message: `${'Warning'.yellow} -- overwrite local with remote ${overwriteType}?`, |
There was a problem hiding this comment.
You can use when option to disable/enable question: https://github.com/SBoudrias/Inquirer.js#question
There was a problem hiding this comment.
Thanks for bringing this to my attention! I have refactored it accordingly.
…ve of final answer
|
@jairo-bc Thanks for your comments! I have tried to address them as best as I can. I made an additional change, which is that I refactored the "overwrite question" to be a confirm, rather than a checkbox. It seemed strange that it was a checkbox since, if my understanding is correct, it's actually just a boolean question, "Should we overwrite the local files?". The only downside I see with that change is that it will be a breaking change of how that prompt behaves. |
What?
I want the ability to run
stencil downloadwithout being prompted about whether to overwrite local files. The goal is to easier be able to run download in my CI pipeline.This is a conceptual pull requests thus far, I have not verified whether the functionality works exactly as intended. This particular command seemed to be untested, so I did not venture to add any tests.
If you have preferences for how this functionality could work different, please let me know or feel free to make changes.