-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
fix(misc): update artifact generator option descriptions and cleanup leftovers #29077
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 30d8774. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 7 targets
Sent with 💌 from NxCloud. |
@@ -9,17 +9,16 @@ | |||
"properties": { | |||
"path": { |
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.
I still think the name path
is confusing, to be honest. To understand what it is, you need to carefully read the description. I think for such a widely used and simple generator, one should not need to read the description. It should be self explanatory. I would argue we keep the name
as the default and prompted property (and infer the path from the name), and leave the path as optional and not prompted. The angular component generator only prompts for name, and I think we should keep that. Maybe I am wrong. I am trying to look at this from an outsider/user perspective.
"description": "The name of the component.", | ||
"x-prompt": "What name would you like to use for the component?" |
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.
I think we should keep this, remove the path
prompt.
"$default": { | ||
"$source": "argv", | ||
"index": 0 | ||
}, | ||
"x-prompt": "Where to create the component?" | ||
"x-prompt": "What is the component file path?" |
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.
Where to generate the component?
?
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.
I considered that one, but it is less explicit about the fact that the option is meant to be a file path. I find it more ambiguous. While the current one explicitly asks for a file path.
c7ddb10
to
0122138
Compare
0122138
to
30d8774
Compare
…leftovers (#29077) - Update artifact generator schemas: - Clarify `path` is the artifact file path relative to the current working directory - Clarify `name` is the artifact symbol name - Remove prompt for `name` and remove it from the important options (won't be displayed by default in Nx Console generation UI, it will be part of the collapsed options) given that most of the time, it's meant to match the filename (last segment of the `path`) - Remove some leftover options related to the name and path formats that were previously missed - Fix an issue with NestJS generators - Fix an issue with Next `page` generator <!-- Please make sure you have read the submission guidelines before posting an PR --> <!-- https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr --> <!-- Please make sure that your commit message follows our format --> <!-- Example: `fix(nx): must begin with lowercase` --> <!-- If this is a particularly complex change or feature addition, you can request a dedicated Nx release for this pull request branch. Mention someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they will confirm if the PR warrants its own release for testing purposes, and generate it for you if appropriate. --> ## Current Behavior <!-- This is the behavior we have today --> ## Expected Behavior <!-- This is the behavior we should expect with the changes in this PR --> ## Related Issue(s) <!-- Please link the issue being fixed so it gets closed when this is merged. --> Fixes # (cherry picked from commit dc67660)
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
path
is the artifact file path relative to the current working directoryname
is the artifact symbol namename
and remove it from the important options (won't be displayed by default in Nx Console generation UI, it will be part of the collapsed options) given that most of the time, it's meant to match the filename (last segment of thepath
)page
generatorCurrent Behavior
Expected Behavior
Related Issue(s)
Fixes #