Skip to content

Update amp-asserts transform to match AmpPass's transformations#28127

Merged
rsimha merged 3 commits intoampproject:masterfrom
jridgewell:amp-asserts
Apr 30, 2020
Merged

Update amp-asserts transform to match AmpPass's transformations#28127
rsimha merged 3 commits intoampproject:masterfrom
jridgewell:amp-asserts

Conversation

@jridgewell
Copy link
Copy Markdown
Contributor

@jridgewell jridgewell commented Apr 30, 2020 •
edited
Loading

This updates amp-asserts babel transform to match the code that AmpPass (which is being removed) generates.

Necessary for #24047
Fixes #23960.

@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Apr 30, 2020

Hey @erwinmombay! These files were changed:

build-system/babel-plugins/babel-plugin-transform-amp-asserts/index.js
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-boolean-non/output.js
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-number-non/output.js
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-dev-assert-string-not-string-non/output.js
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-devAssert-empty-string/output.js
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-devAssert-false/output.js
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-devAssert-null/output.js
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-devAssert-zero/output.js
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-devAssert/output.js
build-system/babel-plugins/babel-plugin-transform-amp-asserts/test/fixtures/transform-assertions/transform-user-fine/output.js

This was referenced Apr 30, 2020
Closed
Merged
rsimha
rsimha approved these changes Apr 30, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up. LGTM!

This fixes #23960, correct?

@rsimha
Copy link
Copy Markdown
Contributor

rsimha commented Apr 30, 2020 •
edited
Loading

Seems like Travis was down earlier today, and the PR job never got kicked off. Perhaps force-push the commits to re-trigger Travis? Or close + re-open this PR.

@rsimha
Copy link
Copy Markdown
Contributor

rsimha commented Apr 30, 2020

Aha, you opened this PR against ampproject/amphtml:amp-asserts, not ampproject/amphtml:master

@jridgewell jridgewell changed the base branch from amp-asserts to master April 30, 2020 03:58
@rsimha
Copy link
Copy Markdown
Contributor

rsimha commented Apr 30, 2020

Bravo, @jridgewell! Merging so I can rebase #24047 on this.

@rsimha rsimha merged commit 12f7bcf into ampproject:master Apr 30, 2020
@jridgewell jridgewell deleted the amp-asserts branch April 30, 2020 04:52
@rsimha
Copy link
Copy Markdown
Contributor

rsimha commented Apr 30, 2020

Celebrated too soon. There still are diffs in several extensions: #24047 (comment)

ldoroshe pushed a commit to ldoroshe/amphtml that referenced this pull request May 8, 2020
@jridgewell @ldoroshe
Update amp-asserts transform to match AmpPass's transformations (ampp…
f07a08f 
…roject#28127)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@rsimha rsimha rsimha approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes PR use: In Beta / Experimental PR use: In Stable

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Not all assert calls are being removed from v0.js by babel transforms

4 participants

@jridgewell @rsimha @googlebot @ampprojectbot