Skip to content

[WIP] turn on collapseProperties#2951

Closed
erwinmombay wants to merge 3 commits intoampproject:masterfrom
erwinmombay:collapse-props
Closed

[WIP] turn on collapseProperties#2951
erwinmombay wants to merge 3 commits intoampproject:masterfrom
erwinmombay:collapse-props

Conversation

@erwinmombay
Copy link
Copy Markdown
Member

No description provided.

@erwinmombay
Copy link
Copy Markdown
Member Author

continuing discussion on this PR. ads.min and everything.min looks good.

@erwinmombay
Copy link
Copy Markdown
Member Author

/cc @cramforce @jridgewell

@jridgewell
Copy link
Copy Markdown
Contributor

Can you post a gist of v0.js and some random extension?

@erwinmombay
Copy link
Copy Markdown
Member Author

@jridgewell so i didnt run v0.js through the collapseProps since something definitely broke, do you want the version that would have been ran through collapse properties?

@erwinmombay
Copy link
Copy Markdown
Member Author

@jridgewell https://gist.github.com/erwinmombay/75c6e289554b7317728f54673b3bb902

contains amp-audio.js and a v0.js transpiled with collapse properties turned on.

@erwinmombay
Copy link
Copy Markdown
Member Author

erwinmombay commented Apr 20, 2016

really interesting to see in amp-audio, it looks like the timer module is duplicated.

@erwinmombay erwinmombay changed the title turn on collapseProperties [WIP] turn on collapseProperties Apr 20, 2016
@erwinmombay
Copy link
Copy Markdown
Member Author

erwinmombay commented Apr 20, 2016

@jridgewell https://gist.github.com/erwinmombay/f4d47e29bde0d391082b88866c28a2b0 slightly hard to read with the suffixes

@erwinmombay
Copy link
Copy Markdown
Member Author

erwinmombay commented Apr 20, 2016

@jridgewell with labels collapsed but with readable names https://gist.github.com/erwinmombay/d2eb139399cc6c6a96fbb8545b8534e6

@cramforce
Copy link
Copy Markdown
Member

Will be interesting to take a deeper look when I have some time. A page
that has just v0.js in it and nothing else, what is the error message?

On Wed, Apr 20, 2016 at 2:13 PM, erwin mombay notifications@github.com
wrote:

@jridgewell https://github.com/jridgewell with labels off
https://gist.github.com/erwinmombay/d2eb139399cc6c6a96fbb8545b8534e6


You are receiving this because you were assigned.
Reply to this email directly or view it on GitHub
#2951 (comment)

@erwinmombay
Copy link
Copy Markdown
Member Author

@cramforce so looking at everything.amp.min.html (and ads.min) with a v0.js that has collaps-props on, i don't see any errors, but its failing on travis consistently with a disconnect (see latest build) and i haven't been able to replicate the same error locally yet but that might also just be because I haven't been able to get a green local test run in a long time now.

@cramforce
Copy link
Copy Markdown
Member

Better than expected :)

On Thu, Apr 21, 2016 at 9:42 AM, erwin mombay notifications@github.com
wrote:

@cramforce https://github.com/cramforce so looking at
everything.amp.min.html (and ads.min) with a v0.js that has collaps-props
on, i don't see any errors, but its failing on travis consistently with a
disconnect (see latest build) and i haven't been able to replicate the same
error locally yet but that might also just be because I haven't been able
to get a green local test run in a long time now.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#2951 (comment)

@erwinmombay
Copy link
Copy Markdown
Member Author

@cramforce closing this in favor of #2972

@erwinmombay erwinmombay reopened this Apr 22, 2016
@erwinmombay
Copy link
Copy Markdown
Member Author

reopening just for travis testing/debugging

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.

3 participants