Skip to content

Quick fix for dep ordering issue in closure compiler with promise polyfill.#1674

Merged
cramforce merged 1 commit intoampproject:masterfrom
cramforce:ie-promise
Jan 29, 2016
Merged

Quick fix for dep ordering issue in closure compiler with promise polyfill.#1674
cramforce merged 1 commit intoampproject:masterfrom
cramforce:ie-promise

Conversation

@cramforce
Copy link
Copy Markdown
Member

Fix is not ideal, but it does come with a regression test.

Fixes #1670

…yfill.

Fix is not ideal, but it does come with a regression test.
@cramforce
Copy link
Copy Markdown
Member Author

Merging. @erwinmombay Please do a post-review.

cramforce added a commit that referenced this pull request Jan 29, 2016
Quick fix for dep ordering issue in closure compiler with promise polyfill.
@cramforce cramforce merged commit cf91b95 into ampproject:master Jan 29, 2016
@cramforce cramforce deleted the ie-promise branch January 29, 2016 16:57
@erwinmombay
Copy link
Copy Markdown
Member

@cramforce slightly concerned with how big the latest file size became (gzipped) https://github.com/ampproject/amphtml/pull/1678/files (i'm assuming that it was caused by this change). will look into it a bit more

@cramforce
Copy link
Copy Markdown
Member Author

@erwinmombay But that is not this change, right? We should binary search when it happened.

@erwinmombay
Copy link
Copy Markdown
Member

@cramforce yeah thats not the exact correct delta, will update in a few minute with exact delta

@erwinmombay
Copy link
Copy Markdown
Member

@cramforce heres the exact delta compared to 1 commit prior (818125b in master)

https://gist.github.com/erwinmombay/7cba4c4c7ade6c059279/revisions

@cramforce
Copy link
Copy Markdown
Member Author

@erwinmombay Ah, understand why the extension regression happened. I was only looking at the amp.js regression which is also unfortunate, but not caused by this. I can try sending an additional change that would fix this.

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.

2 participants