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

build: update to Angular CLI 9.0.0-rc.3 #33955

Closed
wants to merge 2 commits into from

Conversation

filipesilva
Copy link
Contributor

@filipesilva filipesilva commented Nov 21, 2019

Followup to #33337 and angular/angular-cli#16228

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@mary-poppins
Copy link

You can preview 9ef6be5 at https://pr33955-9ef6be5.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 197d758 at https://pr33955-197d758.ngbuilds.io/.

@mary-poppins
Copy link

You can preview b052906 at https://pr33955-b052906.ngbuilds.io/.

"@angular-devkit/architect": "^0.900.0-rc.3",
"@angular-devkit/build-optimizer": "^0.900.0-rc.3",
"@angular-devkit/core": "^9.0.0-rc.3",
"@angular-devkit/schematics": "^9.0.0-rc.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: it seems that @schematics/angular hasn't been updated for a long time as it's still on 8.0.0-beta.15. Can you please update it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@filipesilva filipesilva added the target: patch This PR is targeted for the next patch release label Nov 21, 2019
@filipesilva filipesilva marked this pull request as ready for review November 21, 2019 13:18
@filipesilva filipesilva requested review from a team as code owners November 21, 2019 13:18
@mary-poppins
Copy link

You can preview 3bfeee4 at https://pr33955-3bfeee4.ngbuilds.io/.

@@ -12,7 +12,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2987,
"main-es2015": 506857,
"main-es2015": 464734,
Copy link
Member

@gkalpak gkalpak Nov 21, 2019

Choose a reason for hiding this comment

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

Context:
This was temporarily changed from 461159 to 506857 (pending a cli release) in 08a4f10.
So, there seems to be 3.5KB regression (461159 --> 464734) from rc.1 to rc.3. EDIT: Not true. 👇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hard to say actually. It's true that the commit you linked changed the value. But that wasn't the absolute value, it was a value within +-1% of the real value.

Which is to say, I could have put that value and it would have still passed the CI. To know if there's a regression or not we'd need to check the sizes of FW+CLI rc.2 and FW+CLI rc.3 for AIO.

It's unfortunate that we can't vary just one. Next time we do this sort of coordinated change we should instead leave one of the packages (either FW or CLI) in a intermediate compat mode (@clydin suggested this approach for the change in question, but we ended up not going with it).

Copy link
Member

Choose a reason for hiding this comment

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

You are right. Moreover, this size is for building with the local packages (so neither rc.1 nor rc.3) 😁
So, my comment was invalid 😇

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did both builds:

  • FW+CLI rc.2: 464103
  • FW+CLI rc.3: 464814

So yes, there was a change of 701 bytes. I don't know if it was CLI or the FW though.

@alxhub alxhub added the action: merge The PR is ready for merge by the caretaker label Nov 21, 2019
@alxhub alxhub closed this in 891708c Nov 21, 2019
alxhub pushed a commit that referenced this pull request Nov 21, 2019
alxhub pushed a commit that referenced this pull request Nov 21, 2019
alxhub pushed a commit that referenced this pull request Nov 21, 2019
@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 Dec 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes 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

6 participants