feat: make init command more suitable for automation #634
Conversation
Prompts depend on the absence of cli options. The prompt answers and cli options are then merged. Provided cli options do not count as 'defaultAnswers'.
* feat: init no longer prompts when cli option is present Prompts depend on the absence of cli options. The prompt answers and cli options are then merged. Provided cli options do not count as 'defaultAnswers'. * fix: fix lint errors in lib/stencil-init.js * test: updates integration and unit tests * test: use better naming of methods and constants * fix: reverts change to mock test answers * fix: fix lint errors in lib/stencil-init.spec.js
There was a problem hiding this comment.
I think it's better to add a parameter to bypass prompts.
Another thing is, that for some reason eslint is not validating. Do you have any ideas why? @MaxGenash
|
@NickTolhurst26 thanks for the interest in contributing to this project. From the description of the PR it's said that we are going to make the CLI command more suitable for automation by avoiding user input and using CLI options instead. Are you going to use some testing tools to run CLI commands directly to have real e2e tests? Because at the moment I don’t see any substantial improvements in tests - we are still mocking everything with jest. |
|
Just to note: if we accept this PR, I think we should apply similar changes (don’t ask users to input values that were already provided as CLI options) to all the CLI commands to avoid inconsistency in bahavior |
|
@MaxGenash - I hadn't thought about how to perform e2e tests of a node CLI. Do you know of any particular tools that could achieve this? I've had a browse around and haven't found any strong candidates. I've found a tool called nixt. Is this sort-of what you meant? My use-case for this pull request is that I have a CD pipeline deploying to BigCommerce using Also, I agree that it makes sense to apply these changes across the board. I'll see what I can do. I'm fairly new when it comes to javascript and the |
@MaxGenash - looks like only |
Oh, ok, I got it. I thought by "more 'automatable'" you meant improving testing tools :) |
Yeah, indeed. Moreover, in other CLI commands we already don’t ask users to input values that were already provided as CLI options (e.g. stencil-push --delete, stencil-push --activate ), so stencil-init seems to be the only place to update |
|
@NickTolhurst26 My apologies for the long response, had some other urgent stuff to complete. Generally, the PR LGTM. Left just a few minor remarks and after they are fixed I think it's good to go. Could you also squash your commits into 1, so there is no redundant stuff in the history for such a small change? :) |
- updates JSDoc's that were out-of-date - removes logic out of Rum() method - accept required cliOptions oject at earliest stage
|
@junedkazi reviewed, looks ok for me. If it's ok for you too - let's merge it |
|
Thanks for your time on this guys! And thanks for your advice throughout, @MaxGenash ! What is the release process for this? I'm keen to start using the new feature. Do I just follow the instructions from the
Or is this something one of you guys do? |
|
@NickTolhurst26 we are planning on releasing it early next week. We have a couple more PR we want to see through before we cut a release. |
What?
To make this cli tool more 'automatable' it's necessary to remove the dependency of user input when using the init command.
Rather than having a flag that bypasses user input, it would seem more appropriate to have the cli not ask for values that are already provided via the cli options.
This pull request removes the cli options from the 'defaultAnswers' and uses them to determine which prompts are necessary or not.
It then merges the prompt answers with the cli answers to return the same object as before.
Please let me know if I have done anything incorrect in this contribution process as this is the first time I've contributed to a project - feedback is always welcome.