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

Jump to bottom

build: consolidate @angular-devkit/build-angular to the root package.json #34002

Closed
wants to merge 2 commits into from
Closed

build: consolidate @angular-devkit/build-angular to the root package.json #34002

wants to merge 2 commits into from

Conversation

IgorMinar
Copy link
Contributor

@IgorMinar IgorMinar commented Nov 23, 2019 •
edited by gkalpak

This allows us to update the version of the package in a single place for all tests.

Notable exemption of this is aio which currently doesn't depend on anything installed in the root.

todos:

@IgorMinar IgorMinar requested review from a team as code owners November 23, 2019 00:32
@IgorMinar IgorMinar requested a review from a team as a code owner November 23, 2019 00:57
@mary-poppins
Copy link

You can preview 4338bd9 at https://pr34002-4338bd9.ngbuilds.io/.

gkalpak
gkalpak requested changes Nov 23, 2019
View reviewed changes
Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

CI is a bit unhappy, but otherwise looks great 😃

integration/bazel-schematics/test.sh Outdated Show resolved Hide resolved
integration/cli-hello-world-ivy-compat/package.json Show resolved Hide resolved
@IgorMinar IgorMinar force-pushed the build/consolidate-on-single-build-angular-version branch from 4338bd9 to 4165994 Compare November 26, 2019 17:47
@IgorMinar IgorMinar added the target: patch This PR is targeted for the next patch release label Nov 26, 2019
@mary-poppins
Copy link

You can preview 4165994 at https://pr34002-4165994.ngbuilds.io/.

gkalpak
gkalpak approved these changes Nov 27, 2019
View reviewed changes
Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

Other than #34002 (comment) and the conflicts, lgtm 👌

@IgorMinar IgorMinar force-pushed the build/consolidate-on-single-build-angular-version branch from 4165994 to a38a3ec Compare November 27, 2019 20:22
@IgorMinar
Copy link
Contributor Author

rebased

@mary-poppins
Copy link

You can preview a38a3ec at https://pr34002-a38a3ec.ngbuilds.io/.

@IgorMinar
Copy link
Contributor Author

the ci is still failing. looks like some kind of typings issue :-/

gkalpak
gkalpak reviewed Nov 28, 2019
View reviewed changes
package.json
@@ -134,6 +134,7 @@
"// 3": "when updating @bazel/bazel version you also need to update the RBE settings in .bazelrc (see https://github.com/angular/angular/pull/27935)",
"devDependencies": {
"@angular/cli": "^9.0.0-rc.3",
"@angular-devkit/build-angular": "^0.900.0-rc.3",
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Package managers put this above @angular/cli 😁

integration/bazel-schematics/test.sh Show resolved Hide resolved
@IgorMinar
build: consolidate @angular-devkit/build-angular to the root package.…
d0efedf 
…json

This allows us to update the version of the package in a single place for all tests.

Notable exemption of this is aio which currently doesn't depend on anything installed in the root.
@IgorMinar IgorMinar force-pushed the build/consolidate-on-single-build-angular-version branch from a38a3ec to d0efedf Compare November 29, 2019 06:01
@mary-poppins
Copy link

You can preview d0efedf at https://pr34002-d0efedf.ngbuilds.io/.

@IgorMinar IgorMinar added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Nov 29, 2019
@IgorMinar
Copy link
Contributor Author

merge-assistance: global approval

@IgorMinar IgorMinar added the area: build & ci Related the build and CI infrastructure of the project label Nov 29, 2019
@ngbot ngbot bot added this to the needsTriage milestone Nov 29, 2019
@mary-poppins
Copy link

You can preview 3ad7092 at https://pr34002-3ad7092.ngbuilds.io/.

@petebacondarwin
Copy link
Member

@IgorMinar are you allowed to global approve your own PRs ?? Or is @gkalpak's approval enough to enable your global approval?

josephperrott
josephperrott approved these changes Dec 2, 2019
View reviewed changes
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

@mhevery mhevery closed this in ea37e82 Dec 2, 2019
mhevery pushed a commit that referenced this pull request Dec 2, 2019
@IgorMinar @mhevery
build: consolidate @angular-devkit/build-angular to the root package.…
ca88f7d 
…json (#34002)

This allows us to update the version of the package in a single place for all tests.

Notable exemption of this is aio which currently doesn't depend on anything installed in the root.

PR Close #34002
@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 Jan 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@filipesilva filipesilva filipesilva left review comments

@gkalpak gkalpak gkalpak approved these changes

@clydin clydin clydin left review comments

@petebacondarwin petebacondarwin petebacondarwin approved these changes

@josephperrott josephperrott josephperrott approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: build & ci Related the build and CI infrastructure of the project cla: yes merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
needsTriage
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@IgorMinar @mary-poppins @petebacondarwin @filipesilva @gkalpak @josephperrott @clydin @googlebot