-
Notifications
You must be signed in to change notification settings - Fork 12k
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
feat(@angular/cli): ng doc accepts a version flag #14788
Conversation
Note to the team: this is a first time contributor, contributing to the CLI during the HackCommitPush conference, which aims to help developers to contribute to open source projects :) |
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.
Thank you for the contribution.
Can you add validation of the input?
The option should accept a number or “next” (no “v” prefix for this one). Several numbers are also invalid: 0, 1, and 3.
@clydin What do you think about: "version" : {
"aliases": ["v"],
"anyOf": [
{
"type": "integer",
"minimum": 2
},
{
"const": "next"
}
],
"description": "Contains the version of Angular to use for the documentation."
} and then test for if (options.version) {
if (options.version === 'next') {
domain = 'next.angular.io';
} else if (+options.version !== 3) {
domain = `v${options.version}.angular.io`;
}
} could be good enough for this first PR? |
Since 2 is the outlier, you could add it as a “const” in the anyOf and set the minimum in the integer section to 4. Looks good though. |
Also, if interested in a follow up PR, another nice feature that builds on this would be to automatically use the version installed within the project. |
@clydin Thanks for the feedback! @LakhyariMs Do you think you can update the PR with the changes Charles is suggesting?
and then something like: if (options.version) {
if (options.version === 'next') {
domain = 'next.angular.io';
} else {
domain = `v${options.version}.angular.io`;
}
} |
With this commit, we can now specify a `version` for the `ng doc` command ng doc --version 6 ng doc -v 6 and this will open `v6.angular.io` instead of `angular.io`. The default domain is still used if no version is specified. Refs angular#12365
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.
Thank you again.
I squashed the PR for you so that it passes the validation check.
Also, removed the v
alias as we would like to reserve that for a potential future verbose option on all commands.
Follow-up to angular#14788 that allowed `ng doc --version 6`. This commit enhances the doc command to use the current Angular version of the project by default, if no version is provided explicitely. Fixes angular#12365
Follow-up to angular#14788 that allowed `ng doc --version 6`. This commit enhances the doc command to use the current Angular version of the project by default, if no version is provided explicitely. Fixes angular#12365
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
With this commit, we can now specify a
version
for the
ng doc
commandand this will open
v6.angular.io
instead ofangular.io
.The default domain is still used
if no version is specified.
Refs #12365