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.

Sign up for GitHub

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

Jump to bottom

add a module build explainer #27385

Merged
merged 3 commits into from
Mar 30, 2020
Merged

add a module build explainer #27385

merged 3 commits into from
Mar 30, 2020

Conversation

erwinmombay
Copy link
Member

In preparation to ramp up the module build, we should externally explain at a high level what it is and its details.

@amp-owners-bot amp-owners-bot bot requested a review from mrjoro March 24, 2020 16:40
kevinkimball
kevinkimball reviewed Mar 24, 2020
View reviewed changes
Copy link
Contributor

@kevinkimball kevinkimball left a comment

Choose a reason for hiding this comment

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

Good idea!

erwinmombay
erwinmombay commented Mar 25, 2020
View reviewed changes
AMP_MODULE_BUILD.md Outdated Show resolved Hide resolved
kristoferbaxter
kristoferbaxter approved these changes Mar 26, 2020
View reviewed changes
Copy link
Contributor

@kristoferbaxter kristoferbaxter left a comment

Choose a reason for hiding this comment

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

Awesome! Looks good.

@erwinmombay @kristoferbaxter
Apply suggestions from code review
28bdd49 
Co-Authored-By: Kristofer Baxter <kristofer@kristoferbaxter.com>
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@kristoferbaxter
Copy link
Contributor

CLA was verified. Submissions done via Github UI suggestions.

@erwinmombay
Copy link
Member Author

@mrjoro PTAL when you a get a chance

@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@kristoferbaxter
Copy link
Contributor

@googlebot I fixed it.

@erwinmombay erwinmombay merged commit ea87bcf into ampproject:master Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samouri samouri samouri left review comments

@kevinkimball kevinkimball kevinkimball left review comments

@kristoferbaxter kristoferbaxter kristoferbaxter approved these changes

@powerivq powerivq powerivq approved these changes

@mrjoro mrjoro mrjoro approved these changes

@lannka lannka Awaiting requested review from lannka

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.
8 participants
@erwinmombay @googlebot @kristoferbaxter @powerivq @samouri @kevinkimball @mrjoro @ampprojectbot