Skip to content

feat(@schematics/update): add packageGroup version map support #13124

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

Merged
merged 1 commit into from
Dec 15, 2018

Conversation

hansl
Copy link
Contributor

@hansl hansl commented Dec 3, 2018

Fixes #13015

@ngbot
Copy link

ngbot bot commented Dec 6, 2018

Hi @hansl! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

@hansl hansl force-pushed the fix-13015 branch 2 times, most recently from 32fbbca to 3d011bd Compare December 6, 2018 22:13
}
}

if (typeof metadata['packageGroupName'] == 'string') {
Copy link
Member

Choose a reason for hiding this comment

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

This conditional set can be removed with the above changes.

@hansl
Copy link
Contributor Author

hansl commented Dec 10, 2018

@clydin PTAL.

"ng-update": {
"packageGroup": {
"@angular-devkit-tests/update-package-group-1": "^1",
"@angular-devkit-tests/update-package-group-2": ""
Copy link
Member

Choose a reason for hiding this comment

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

Specifying the owning package again in the group and with an empty version specifier is ambiguous. From an npm perspective this would mean install "any version" but the user doesn't want "any version". They want the version of the actual package specified above. They could put the version there as well but then there are two elements that need to be kept in sync within the same file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Angular does it and it's kind of suggested (since it's just easier to list all packages in all package.json). It's supported and it's ignored since it's on the command line.

@mgechev mgechev added the target: major This PR is targeted for the next major release label Dec 15, 2018
@mgechev mgechev merged commit 851d694 into angular:master Dec 15, 2018
@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 12, 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.

As a CLI user, I want ng update to also update dependencies IF they're present.
4 participants