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

I2I: Deprecate attribute cross-origin in favor of crossorigin for remote XHR identity requests. #21399

Closed
7 tasks done
hellokoji opened this issue Mar 13, 2019 · 14 comments
Closed
7 tasks done
Assignees

Comments

@hellokoji
Copy link
Contributor

hellokoji commented Mar 13, 2019

In accordance with the deprecation policy.

With reference to I2I Issue (#18787) and Pull Request (#21107), the attribute cross-origin and crossorigin will both be supported to enable POST based identity on remote XHR requests. This deprecation will remove cross-origin as a valid attribute in favor of crossorigin to align with the standard CORS attribute.

  • Allow for at least 2 weeks of open discussion.
  • Wait for 3 approvals from core committers.
  • Update spec
  • Announce change on the mailing list.
  • Start warning for pages that might break via the developer console.
  • Give developers 6 weeks to apply changes.
  • Update validator
@torch2424
Copy link
Contributor

closing this as this was implemented 😄

Feel free to comment if this needs more work.

@honeybadgerdontcare
Copy link
Contributor

@hellokoji Confirming that all the above steps have been completed before approving #21909. It's just been about six weeks for developers to apply, but I don't see any approvals from core committers here and was there an announcement on the mailing list?

@honeybadgerdontcare
Copy link
Contributor

@ampproject/wg-approvers for 3 approvals from core committers.

@honeybadgerdontcare
Copy link
Contributor

@hellokoji please announce to amphtml-announce@googlegroups.com

@dvoytenko
Copy link
Contributor

LGTM from me, but I agree with @honeybadgerdontcare. Let's assign the I2I to @ampproject/wg-approvers from now on.

@dvoytenko
Copy link
Contributor

/to @choumx and @Gregable to provide additional approvals.

@hellokoji
Copy link
Contributor Author

Apologies, I must have missed this. You're right, this never got announced on the mailing list and never had core committer approval.

I'll send out the announcement on the amphtml-announce@googlegroups.com mailing list.

@hellokoji
Copy link
Contributor Author

Sent.

@Gregable
Copy link
Member

I don't think this change has much to do with wg-caching, other than the fact that it is a (small) literal change to validator rules. Perhaps @cramforce instead for the additional approval.

@dreamofabear
Copy link

LGTM

@honeybadgerdontcare
Copy link
Contributor

@Gregable this isn't about wg-caching but instead about getting 3 approvals from wg-approvers which you are a member of. So far Dima and Will approved. You could be the third to move this along.

@hellokoji
Copy link
Contributor Author

Friendly ping to this thread. Still need 1 more approval from a core-committer.

@cramforce
Copy link
Member

Approved

@hellokoji
Copy link
Contributor Author

Marking closed with #21909 being merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants