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.

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

🏗 Add APPROVERS.json to enable blocking bundle-size checks for extensions #25652

Merged
merged 6 commits into from Nov 20, 2019

Conversation

danielrozenberg
Copy link
Member

wip - not yet being used because the bundle-size app needs to be updated to use it

Copy link
Contributor

@rcebulko rcebulko left a comment

Choose a reason for hiding this comment

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

Left a couple comments but otherwise LGTM

build-system/tasks/bundle-size/APPROVERS.json Show resolved Hide resolved
build-system/tasks/bundle-size/README.md Outdated Show resolved Hide resolved
build-system/tasks/bundle-size/README.md Outdated Show resolved Hide resolved
build-system/tasks/bundle-size/README.md Outdated Show resolved Hide resolved
@danielrozenberg
Copy link
Member Author

danielrozenberg commented Nov 19, 2019

Addressed your comments. Please take another look, wanna get this right :)

Copy link
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.

Nice PR!

I've requested a few changes below. Mostly grammar nits, and one question about file formats.

build-system/tasks/bundle-size/APPROVERS.json Show resolved Hide resolved
build-system/tasks/bundle-size/README.md Outdated Show resolved Hide resolved
build-system/tasks/bundle-size/README.md Show resolved Hide resolved
build-system/tasks/bundle-size/README.md Show resolved Hide resolved
build-system/tasks/bundle-size/README.md Outdated Show resolved Hide resolved
build-system/tasks/bundle-size/README.md Outdated Show resolved Hide resolved
build-system/tasks/bundle-size/APPROVERS.json Show resolved Hide resolved
Copy link
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 after one more comment 👍

build-system/tasks/bundle-size/README.md Outdated Show resolved Hide resolved
@danielrozenberg danielrozenberg merged commit ddd59f4 into ampproject:master Nov 20, 2019
@danielrozenberg danielrozenberg deleted the approvers.json branch November 20, 2019 15:51
micajuine-ho pushed a commit to micajuine-ho/amphtml that referenced this pull request Dec 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants