Skip to content

🏗 Use native closure compiler with pre-compiled AMP runner#29099

Merged
kristoferbaxter merged 4 commits intoampproject:masterfrom
kristoferbaxter:native-custom-closure
Jul 1, 2020
Merged

🏗 Use native closure compiler with pre-compiled AMP runner#29099
kristoferbaxter merged 4 commits intoampproject:masterfrom
kristoferbaxter:native-custom-closure

Conversation

@kristoferbaxter
Copy link
Copy Markdown
Contributor

@kristoferbaxter kristoferbaxter commented Jul 1, 2020

This switches from nailgun to a native closure compiler implementation including the custom AMP Command Line Runner.

@amp-owners-bot amp-owners-bot bot requested a review from rsimha July 1, 2020 00:15
@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Jul 1, 2020

Hey @rsimha! These files were changed:

build-system/compile/closure-compile.js
build-system/compile/compile.js
build-system/runner/OWNERS
build-system/runner/build.xml
build-system/runner/src/org/ampproject/AmpCodingConvention.java
build-system/runner/src/org/ampproject/AmpCommandLineRunner.java

Hey @erwinmombay! These files were changed:

build-system/runner/OWNERS
build-system/runner/build.xml
build-system/runner/src/org/ampproject/AmpCodingConvention.java
build-system/runner/src/org/ampproject/AmpCommandLineRunner.java

ThwalT
ThwalT approved these changes Jul 1, 2020
View reviewed changes
Copy link
Copy Markdown

@ThwalT ThwalT left a comment

Choose a reason for hiding this comment

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

@

rsimha
rsimha approved these changes Jul 1, 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.

LGTM. Thank you for making this work!

Suggested issue annotations (you might need to copy these to the PR description for them to take effect):

Closes #28172
Closes #28173
Fixes #28208

This was linked to issues Jul 1, 2020
Remove AmpCodingConvention.java #28172
Closed
Remove AmpCommandLineRunner.java #28173
Closed
Use native closure compiler instead of java runner.jar #28208
Closed
@rsimha rsimha mentioned this pull request Jul 1, 2020
Closed
@kristoferbaxter kristoferbaxter merged commit 242e092 into ampproject:master Jul 1, 2020
@kristoferbaxter kristoferbaxter deleted the native-custom-closure branch July 1, 2020 19:15
@rsimha rsimha mentioned this pull request Aug 26, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@rsimha rsimha rsimha approved these changes

@ThwalT ThwalT ThwalT approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes PR use: In Beta / Experimental

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Use native closure compiler instead of java runner.jar Remove AmpCommandLineRunner.java Remove AmpCodingConvention.java

5 participants

@kristoferbaxter @rsimha @ThwalT @googlebot @ampprojectbot