Skip to content

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

Closed
filipesilva wants to merge 2 commits into
angular:masterfrom
filipesilva:cli-rc.3
Closed

build: update to Angular CLI 9.0.0-rc.3#33955
filipesilva wants to merge 2 commits into
angular:masterfrom
filipesilva:cli-rc.3

Conversation

@filipesilva
Copy link
Copy Markdown
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
Copy Markdown

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

@mary-poppins
Copy link
Copy Markdown

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

@mary-poppins
Copy link
Copy Markdown

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

Comment thread package.json
"@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
Copy Markdown
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
Copy Markdown
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 requested a review from alxhub November 21, 2019 13:18
@filipesilva filipesilva marked this pull request as ready for review November 21, 2019 13:18
@filipesilva filipesilva requested review from a team November 21, 2019 13:18
@mary-poppins
Copy link
Copy Markdown

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

"uncompressed": {
"runtime-es2015": 2987,
"main-es2015": 506857,
"main-es2015": 464734,
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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.

6 participants