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.

Sign up for GitHub

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

Update Angular to use 2.6 #21144

Closed
wants to merge 1 commit into from
Closed

Update Angular to use 2.6 #21144

wants to merge 1 commit into from

Conversation

chuckjaz
Copy link
Contributor

PR Type

What kind of change does this PR introduce?

[x] Refactoring (no functional changes, no api changes)

What is the current behavior?

Issue Number: #20653

Angular uses 2.5 to build.

What is the new behavior?

Angular uses 2.6 to build.

Also updates the dependencies to support using 2.6.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@chuckjaz chuckjaz added cla: yes and removed cla: no labels Dec 21, 2017
@mary-poppins
Copy link

You can preview 9655f9a at https://pr21144-9655f9a.ngbuilds.io/.

@chuckjaz chuckjaz force-pushed the update-ts-26 branch 2 times, most recently from 70aa772 to 2bf3723 Compare December 22, 2017 00:32
@mary-poppins
Copy link

You can preview 70aa772 at https://pr21144-70aa772.ngbuilds.io/.

@chuckjaz chuckjaz mentioned this pull request Dec 22, 2017
Closed
@mary-poppins
Copy link

You can preview 2bf3723 at https://pr21144-2bf3723.ngbuilds.io/.

alan-agius4
alan-agius4 reviewed Dec 22, 2017
View reviewed changes
packages/compiler-cli/package.json Outdated
@@ -11,11 +11,11 @@
"dependencies": {
"reflect-metadata": "^0.1.2",
"minimist": "^1.2.0",
"tsickle": "^0.25.5",
"tsickle": "^0.25.6",
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be ^0.26.0 since when there is no major version ^ is equal to ~ and in https://github.com/angular/angular/pull/21144/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R99

you are using 0.26.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Nice catch!

@mary-poppins
Copy link

You can preview ef9392b at https://pr21144-ef9392b.ngbuilds.io/.

@chuckjaz chuckjaz force-pushed the update-ts-26 branch 2 times, most recently from 4aee016 to dee0cb1 Compare December 22, 2017 18:54
@mary-poppins
Copy link

You can preview 4aee016 at https://pr21144-4aee016.ngbuilds.io/.

@mary-poppins
Copy link

You can preview dee0cb1 at https://pr21144-dee0cb1.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 84e7663 at https://pr21144-84e7663.ngbuilds.io/.

@mary-poppins
Copy link

You can preview cb4b6fc at https://pr21144-cb4b6fc.ngbuilds.io/.

@chuckjaz chuckjaz added area: build & ci Related the build and CI infrastructure of the project action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release labels Dec 22, 2017
@IgorMinar IgorMinar added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 22, 2017
@IgorMinar
Copy link
Contributor

I rebased this and force pushed

@IgorMinar IgorMinar mentioned this pull request Dec 22, 2017
Closed
3 tasks
@mary-poppins
Copy link

You can preview 2e4f752 at https://pr21144-2e4f752.ngbuilds.io/.

clydin
clydin reviewed Dec 23, 2017
View reviewed changes
packages/compiler-cli/src/ngtools_api2.ts Outdated
@@ -146,4 +146,4 @@ export function createCompilerHost(
export type Diagnostics = Array<ts.Diagnostic|Diagnostic>;
Copy link
Member

Choose a reason for hiding this comment

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

Convert to ReadonlyArray as well?

@IgorMinar
Copy link
Contributor

I updated the size limit since the uncompressed main bundle is now smaller by 4kb (likely due to the cli update). And I force pushed with lease.

@mary-poppins
Copy link

You can preview 4267d0d at https://pr21144-4267d0d.ngbuilds.io/.

@mary-poppins
Copy link

You can preview d66586c at https://pr21144-d66586c.ngbuilds.io/.

clydin
clydin approved these changes Dec 23, 2017
View reviewed changes
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.

CLI related ngtools_api changes reviewed

@IgorMinar
Copy link
Contributor

@alxhub Chuck made a few changes to the service worker code while upgrading to typescript 2.6. I’m assuming that you are not around to review them, so I’m going to land his PR now but I’d like you to take a look at those changes and do a post-submit review and let us know if this is not the right change for some reason. The two hunks to look at are:

https://github.com/angular/angular/pull/21144/files#diff-fd848aaa6003a291716c374486be73e6L28
https://github.com/angular/angular/pull/21144/files#diff-6f9f8bc35a9ef23c9f5ae1b28a164b59L77

@IgorMinar
Copy link
Contributor

I had to update the plugin-typescript for systemjs integration test to go green, so I force pushed one more time to this branch

@mary-poppins
Copy link

You can preview 78eff64 at https://pr21144-78eff64.ngbuilds.io/.

@IgorMinar
Copy link
Contributor

ci is green except for a flake

@IgorMinar IgorMinar closed this in 83d207d Dec 23, 2017
@gkalpak gkalpak mentioned this pull request Jan 4, 2018
Closed
3 tasks
@sarunint sarunint mentioned this pull request Jan 6, 2018
Closed
@vicb vicb mentioned this pull request Jan 17, 2018
Closed
@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.
Reviewers

@alan-agius4 alan-agius4 alan-agius4 left review comments

@IgorMinar IgorMinar IgorMinar approved these changes

@clydin clydin clydin 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 target: major This PR is targeted for the next major release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@chuckjaz @googlebot @mary-poppins @IgorMinar @alan-agius4 @clydin