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

📖Fix typo in AMP Variable Substitutions documentation #24818

Merged
merged 1 commit into from
Oct 16, 2019
Merged

📖Fix typo in AMP Variable Substitutions documentation #24818

merged 1 commit into from
Oct 16, 2019

Conversation

mattwomple
Copy link
Member

<amp-pixel> was missing > on its opening tag.

<amp-pixel> was missing ">" on its opening tag.
@mattwomple
Copy link
Member Author

CC @ampproject/wg-outreach

@CrystalOnScript
Copy link
Contributor

Thanks for the fix!

@mattwomple
Copy link
Member Author

Anything delaying merge on this one? Thanks!

@mrjoro mrjoro closed this Oct 16, 2019
@mrjoro mrjoro reopened this Oct 16, 2019
Copy link
Member

@mrjoro mrjoro left a comment

Choose a reason for hiding this comment

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

Adding a review comment to restart the owners bot!

Copy link
Member

@mrjoro mrjoro left a comment

Choose a reason for hiding this comment

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

Oh and approving since it needs a Reviewer approval.

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.

Test comment

@rcebulko
Copy link
Contributor

The owners bot is WAI. This PR does not have owners approval (spec has no OWNERS files, so this requires one of the top-level owners). It's possible ampproject/wg-outreach should own all MD files (not just those in root), which #25075 does.

@mrjoro
Copy link
Member

mrjoro commented Oct 16, 2019

Once #25075 goes in, @CrystalOnScript's approval will count as an OWNER approval, so it should be ready to merge.

Thanks for your patience as we work through some of the last issues with the Owners/Reviewers checks!

@rcebulko rcebulko self-requested a review October 16, 2019 19:47
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.

rerun owners

@mrjoro
Copy link
Member

mrjoro commented Oct 16, 2019

Woo! It looks like it's ready to merge now. @mattwomple use your powers!

@mattwomple
Copy link
Member Author

Woo! It looks like it's ready to merge now. @mattwomple use your powers!

Thanks @mrjoro but I don't think Collaborators actually have write access. I still see "The base branch restricts merging to authorized users." and the email from github reads:

You’ve been added to the Collaborators (amphtml) team for the AMP organization. Collaborators (amphtml) has 13 members and gives *read* access to 2 ampproject repositories.

@mrjoro
Copy link
Member

mrjoro commented Oct 16, 2019

OK, I think it should (hopefully) work now... there was one more setting we needed to adjust. :)

If it still doesn't work, we may need to wait for the permissions to propagate. But let's see...

@mattwomple mattwomple merged commit f515760 into ampproject:master Oct 16, 2019
@mattwomple mattwomple deleted the patch-2 branch October 16, 2019 20:21
@rsimha
Copy link
Contributor

rsimha commented Oct 16, 2019

I love the entire sequence of events in this PR! Glad to have you as a collaborator, @mattwomple!

joshuarrrr pushed a commit to Parsely/amphtml that referenced this pull request Oct 22, 2019
micajuine-ho pushed a commit to micajuine-ho/amphtml that referenced this pull request Dec 27, 2019
<amp-pixel> was missing ">" on its opening tag.
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.

6 participants