Navigation Menu

Skip to content
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

Merged
merged 1 commit into from Jun 17, 2019

Conversation

LakhyariMs
Copy link
Contributor

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 #12365

@cexbrayat
Copy link
Member

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 :)

Copy link
Member

@clydin clydin left a 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 clydin added the target: major This PR is targeted for the next major release label Jun 15, 2019
@cexbrayat
Copy link
Member

@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 3 in code (I don't think we can exclude 3 in the schema?)
Ideally, we should also check that the version is not greater than the latest released, but that might be trickier. So I guess the logic:

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?

@clydin
Copy link
Member

clydin commented Jun 16, 2019

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.

@clydin
Copy link
Member

clydin commented Jun 16, 2019

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.

@cexbrayat
Copy link
Member

@clydin Thanks for the feedback!
Yes we were thinking about it, but I was unsure about the best way to get the Angular version installed in the project. What would you recommend?

@LakhyariMs Do you think you can update the PR with the changes Charles is suggesting?
Something like:

"version" : {
  "aliases": ["v"],
  "anyOf": [
    {
       "const": 2
    },
    {
      "type": "integer",
      "minimum": 4
    },
    {
      "const": "next"
    }
  ],
  "description": "Contains the version of Angular to use for the documentation."
}

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
Copy link
Member

@clydin clydin left a 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.

@mgechev mgechev merged commit 58599e1 into angular:master Jun 17, 2019
cexbrayat added a commit to cexbrayat/angular-cli that referenced this pull request Jun 27, 2019
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
cexbrayat added a commit to cexbrayat/angular-cli that referenced this pull request Jun 27, 2019
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
vikerman pushed a commit that referenced this pull request Jun 27, 2019
Follow-up to #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 #12365
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants