Skip to content

Conversation

@yjbanov
Copy link
Contributor

@yjbanov yjbanov commented Dec 15, 2015

This commit reverts a8d9dbf that introduced a code size regression (16kb gzipped, 63kb minified) in Dart.

Effect on the hello world app:

gzipped: 105kb -> 89kb
minified: 370kb -> 317kb

BREAKING CHANGE:

  • This is very unlikely to be breaking, but I'm still marking just in case. The only change to the user should be that dev mode is driven by Dart's checked mode, like it was in the past.

Closes #5883
Closes #5888

@yjbanov
Copy link
Contributor Author

yjbanov commented Dec 15, 2015

/cc @vsavkin @IgorMinar

Copy link
Contributor

Choose a reason for hiding this comment

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

please don't change limits in a drive-by way. in the future please create separate commit and explain why you are decreasing the limit.

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

@IgorMinar
Copy link
Contributor

I believe that I saw some tests in g3 that required dev mode even when not running in dartium's checked mode.

can you please verify how to deal with this (by cs-ing enableDevMode) before we merge this in?

@IgorMinar IgorMinar added this to the beta.0 milestone Dec 15, 2015
IgorMinar added a commit to IgorMinar/angular that referenced this pull request Dec 15, 2015
This is inconsistent with TS/JS, but if set to true dart2js tree-shaking doesn't work and our e2e tests time out.

Related to angular#5896
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@yjbanov yjbanov force-pushed the codesize branch 2 times, most recently from 9422eb4 to 75b20ff Compare December 15, 2015 08:15
@IgorMinar
Copy link
Contributor

this PR was still failing on CI even after rebasing on top for @vsavkin's PR.

@vsavkin landed 3dca9d5 in the meantime, can you please rebase?

@IgorMinar IgorMinar modified the milestones: beta.1, beta.0 Dec 15, 2015
This commit reverts a8d9dbf that introduced a code size regression (16kb gzipped, 63kb minified) in Dart.

Effect on the hello world app:

gzipped: 105kb -> 89kb
minified: 370kb -> 317kb

BREAKING CHANGE:
- This is very unlikely to be breaking, but I'm still marking just in case. The only change to the user should be that dev mode is driven by Dart's checked mode, like it was in the past.
Our code size SLA is 100kb gzipped and 300kb minified. Our target is 10kb gzipped.

We are currently under 90kb gzipped. Let's keep it that way while looking for ways to improve the situation further. We're not <300kb minified yet, so we should be stricter here too.
@googlebot
Copy link

CLAs look good, thanks!

@IgorMinar IgorMinar modified the milestone: beta.1 Dec 15, 2015
@yjbanov yjbanov added action: review The PR is still awaiting reviews from at least one requested reviewer action: merge The PR is ready for merge by the caretaker and removed action: discuss labels Dec 17, 2015
@alexeagle alexeagle added zomg_admin: do merge and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 18, 2015
@mary-poppins
Copy link

Merging PR #5896 on behalf of @alexeagle to branch presubmit-alexeagle-pr-5896.

@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 7, 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Turn on dev mode by default in dev bundles

7 participants