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

docs(@angular/cli): add --location=global flag for Nodejs v16 and later #23772

Closed
wants to merge 1 commit into from

Conversation

estrellajm
Copy link
Contributor

@estrellajm estrellajm commented Aug 21, 2022

docs(@angular/cli): add --location=global flag for Nodejs v16 and later.

When installing @angular/cli with the global flag -g for Nodejs v16 and later it throws a warning. Added verbiage to show both flags. Nodejs v14 and older users can continue to use the -g flag. Nodejs v16 and later users need to use the --location=global flag.

UPDATE: update to README.md.

@alan-agius4
Copy link
Collaborator

Thanks for this however NPM 14 is still supported. Let's update this when Node.js 14 support is dropped.

@estrellajm
Copy link
Contributor Author

Thanks for this however NPM 14 is still supported. Let's update this when Node.js 14 support is dropped.

I still maintained the original command and just listed both ways.

So v16 could use the new, v14 would still see the old.

@alan-agius4
Copy link
Collaborator

Oops didn't see that.

Can you update the commit message as per guidelines https://github.com/angular/angular-cli/blob/main/CONTRIBUTING.md#commit

@alan-agius4 alan-agius4 reopened this Aug 22, 2022
@alan-agius4 alan-agius4 added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews target: patch This PR is targeted for the next patch release labels Aug 22, 2022
@estrellajm
Copy link
Contributor Author

estrellajm commented Aug 22, 2022 via email

@estrellajm
Copy link
Contributor Author

@alan-agius4 updated the commit message.

@alan-agius4
Copy link
Collaborator

@estrellajm, I looks like only the PR description was updated.

@estrellajm
Copy link
Contributor Author

Oops! one sec

@estrellajm estrellajm changed the title Update README.md docs(@angular/cli): add --location=global flag for Nodejs v16 and later Aug 22, 2022
@estrellajm
Copy link
Contributor Author

Ok, I think I got everything. Updated commit message and PR.

@alan-agius4
Copy link
Collaborator

Can you squash so that there is a single commit please? Thanks.

@estrellajm
Copy link
Contributor Author

estrellajm commented Aug 22, 2022

@alan-agius4 Ok, I think I got it. Thanks for the help on this. I'll remember these items for my next PR

Please let me know if there are any other clean up items that I need to address

@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Aug 22, 2022
@ngbot
Copy link

ngbot bot commented Aug 22, 2022

I see that you just added the action: merge label, but the following checks are still failing:
    failure status "ci/circleci: validate" is failing
    pending forbidden labels detected: action: cleanup
    pending status "ci/circleci: e2e-cli-esbuild" is pending
    pending status "ci/circleci: e2e-cli-npm" is pending
    pending status "ci/circleci: e2e-cli-win" is pending
    pending status "ci/circleci: e2e-cli-yarn" is pending
    pending status "ci/circleci: test" is pending
    pending status "ci/circleci: test-browsers" is pending
If you want your PR to be merged, it has to pass all the CI checks.
If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Thanks for your contribution.

@alan-agius4 alan-agius4 removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews action: merge The PR is ready for merge by the caretaker labels Aug 22, 2022
@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Aug 22, 2022
@clydin clydin added needs: discussion On the agenda for team meeting to determine next steps and removed action: merge The PR is ready for merge by the caretaker labels Aug 22, 2022
@clydin
Copy link
Member

clydin commented Aug 22, 2022

The --global flag was mistakenly deprecated in version 8.11. The deprecation was reverted in version 8.12.1 (npm/cli#4982).

@estrellajm
Copy link
Contributor Author

@clydin thank you! I just upgraded my npm to v8.15 and can confirm!

I must say, this is good news. Closing this PR without merge or further action.

@estrellajm estrellajm closed this Aug 22, 2022
@clydin clydin removed the needs: discussion On the agenda for team meeting to determine next steps label Aug 22, 2022
@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 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants