-
-
Notifications
You must be signed in to change notification settings - Fork 75
feat(cli): add promptless args to README.md
#864
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
Conversation
🦋 Changeset detectedLatest commit: 669aaf1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
|
Looks like the tests are failing. You can easily reproduce this by running Apart from that I will leave this open for @jycouet so that he can check if and how this will conflict with the changes from #684 |
|
There are still failing tests, but I'm going to leave it as is until @jycouet gives it a pass, as this may be the wrong approach. |
jycouet
left a comment
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.
Tests are failing on windows because of / or \
The issue is more deep than just the args stuff. it's the [path] that should use something like path.posix
I just tested on windows and npx sv create hello/you is working. So we should make it / in all OS imo.
packages/sv/lib/cli/add/index.ts
Outdated
| if (fromCommand === 'add') common.logArgs(packageManager, 'add', argsFormattedAddons); | ||
| if (fromCommand === 'add') { | ||
| const prompt = common.buildArgs(packageManager, 'add', argsFormattedAddons); | ||
| p.log.message(`To rerun this command without prompts, run:\n${pc.dim(prompt)}`); |
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.
Look at the other comment for location.
Having a small text on top, why not... But very small imo. (I did a few when the feature was crafted, but didn't find something nice)
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.
What about:
- To re-run without prompts:
- Reproducible CLI command:
- Non-interactive:
- Automate with:
- Skip prompts:
- Run again:
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.
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.
- Re-run without questions
- Re-run without prompts
- Run again, no questions asked
- Run again, no prompts
I this that the text should by gray, maybe try with log info as well to have a dot in timeline ? (I think I didn't like it much, but with a title maybe... Maybe not 😅
This reverts commit fef505e.
jycouet
left a comment
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.
The readme is not updated when we do just a add cmd in a second step. It's probably "normal", but I would like to note it here.
jycouet
left a comment
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.
Look OK to me now


closes #863
I cannot reuse the process built into
createKitto updateREADME.mdas the order of operation iscreateKit->runAddonsApply->buildArgs, at that point it is too late to usecreateKit. Thus, a manual approach was used to update theREADME.md.