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

Bump glimmer-vm packages to 0.24. #15245

Merged
merged 38 commits into from
May 29, 2017
Merged

Bump glimmer-vm packages to 0.24. #15245

merged 38 commits into from
May 29, 2017

Conversation

chancancode
Copy link
Member

@chancancode chancancode commented May 17, 2017

@homu
Copy link
Contributor

homu commented May 17, 2017

☔ The latest upstream changes (presumably #15247) made this pull request unmergeable. Please resolve the merge conflicts.

@homu
Copy link
Contributor

homu commented May 23, 2017

☔ The latest upstream changes (presumably #15254) made this pull request unmergeable. Please resolve the merge conflicts.

rwjblue added 4 commits May 22, 2017 21:35
This is only needed when a catastrophic failure happens that
prevents the normal removal from working. Doing this in the
teardown greatly reduces the number of invalid faulures when
errors occur during rendering.
These are largely duplication functionality in `{{each-in}}`'s own
tests.  They were added as a way to flesh out the iterable implementation.
@knownasilya
Copy link
Contributor

Interesting seeing the dog fooding of component-managers and how they are used. Good stuff and much to learn.

@rwjblue rwjblue changed the title [WIP] Bump glimmer Bump glimmer-vm packages to 0.24. May 23, 2017
@rwjblue
Copy link
Member

rwjblue commented May 24, 2017

Did some quick asset size checks, and the min + gz size is slightly better (current master 120.54 KB; after this PR 120.15 KB).

@rwjblue
Copy link
Member

rwjblue commented May 26, 2017

I did some general benchmarking (against the emberaddons.com app), and it seems that we haven't introduced a regression here.

phases

Seems good to ship...

@knownasilya
Copy link
Contributor

:shipit:

@rwjblue
Copy link
Member

rwjblue commented May 27, 2017

Did another test against emberobserver.com (which has a larger rendering phase):

phases

}
}

didCreateElement() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't these go on the abstract manager?

return null;
}

didCreateElement() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about putting them on the abstract manager.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, totally agreed.

@chadhietala
Copy link
Contributor

LGTM

@rwjblue rwjblue merged commit 91d8f08 into master May 29, 2017
@knownasilya
Copy link
Contributor

Woop

@locks locks deleted the bump-glimmer branch July 18, 2017 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants