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

🌸 Cherry-pick request for #28994 into #2006152249000 (Pending) #28989

Closed
zhouyx opened this issue Jun 22, 2020 · 5 comments
Closed

🌸 Cherry-pick request for #28994 into #2006152249000 (Pending) #28989

zhouyx opened this issue Jun 22, 2020 · 5 comments
Assignees
Labels
Cherry-pick: Beta Cherry-pick: Experimental Type: Release Used to track AMP releases from canary to production

Comments

@zhouyx
Copy link
Contributor

zhouyx commented Jun 22, 2020

Cherry-pick request

Issue PR Beta / Experimental? Stable? LTS? Release issue
#28994 #28988 Yes No No #<RELEASE_ISSUE>

Why does this issue meet the cherry-pick criteria?

The issue is causing us from reporting visibility even if the AMPHTML ad is below the folder when the iframeClient doesn't exist (rendered in a friendly iframe)

Mini-postmortem

TODO: This postmortem will be written after the cherry-pick deployment and before this issue is closed. Delete this TODO when the postmortem is ready.

Summary

<1-2 sentences summarizing the problem and root causes.>

Impact

  • <Which users were affected? Roughly how many?>
  • <How were users affected? E.g. partial or complete loss of functionality?>

Action Items

  • #<ISSUE_NUMBER>: <Add unit/integration/end-to-end test>
  • #<ISSUE_NUMBER>: <Add monitoring for edge case via error logging>
  • #<ISSUE_NUMBER>: <Refactor an easily misused API>

/cc @ampproject/release-on-duty @ampproject/wg-approvers @ampproject/cherry-pick-approvers

@zhouyx
Copy link
Contributor Author

zhouyx commented Jun 22, 2020

The ads team used 2006152249000 as RC, which is not RC for AMP docs (2006112352000). Maybe we could cherrypick to 2006112352000 instead and have them changed RC version to that.

@dvoytenko
Copy link
Contributor

Approved.

@zhouyx
Copy link
Contributor Author

zhouyx commented Jun 23, 2020

to @jeffkaufman and @jridgewell
Since the issue is inabox only, we can do it either way

  1. Perform a cherrypick to nightly build 2006152249000. (inabox RC version)
    or
  2. Perform a RC cherrypick to 2006112352000 (AMP RC version), and set the inabox RC to this version.

@zhouyx zhouyx changed the title 🌸 Cherry-pick request for #28988 into #2006152249000 (Pending) 🌸 Cherry-pick request for #28994 into #2006152249000 (Pending) Jun 23, 2020
@jeffkaufman
Copy link
Contributor

We would be happy with either (1) or (2). Whichever you prefer!

Thanks for fixing this!

@zhouyx
Copy link
Contributor Author

zhouyx commented Jun 23, 2020

Instead, we agreed to proceed with the new nightly build 2006230309000. Closing the cherry-pick request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cherry-pick: Beta Cherry-pick: Experimental Type: Release Used to track AMP releases from canary to production
Projects
None yet
Development

No branches or pull requests

5 participants