Skip to content

Remove deprecated Cloudflare AMP Cache and A4A support.#26912

Merged
jridgewell merged 1 commit intoampproject:masterfrom
cloudflare:oli/remove-cloudflare
Mar 10, 2020
Merged

Remove deprecated Cloudflare AMP Cache and A4A support.#26912
jridgewell merged 1 commit intoampproject:masterfrom
cloudflare:oli/remove-cloudflare

Conversation

@oliy
Copy link
Copy Markdown
Contributor

@oliy oliy commented Feb 22, 2020

No description provided.

@amp-owners-bot amp-owners-bot bot requested a review from jridgewell February 22, 2020 00:17
@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Feb 22, 2020 •
edited
Loading

Hey @ampproject/wg-caching, these files were changed:

validator/engine/validator-in-browser.js

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Feb 22, 2020

This pull request introduces 1 alert and fixes 2 when merging 108642a into 99c5984 - view on LGTM.com

new alerts:

  • 1 for Syntax error

fixed alerts:

  • 2 for Incomplete URL substring sanitization

@oliy oliy force-pushed the oli/remove-cloudflare branch from 108642a to 9c1eb10 Compare February 22, 2020 22:41
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Feb 22, 2020

This pull request fixes 2 alerts when merging 9c1eb10 into 4c71f6c - view on LGTM.com

fixed alerts:

  • 2 for Incomplete URL substring sanitization

@jridgewell
Copy link
Copy Markdown
Contributor

jridgewell commented Feb 24, 2020

/to @ampproject/wg-caching

jridgewell
jridgewell reviewed Feb 24, 2020
View reviewed changes
validator/engine/validator-in-browser.js Outdated
return (
url.toLowerCase().indexOf('cdn.ampproject.org') !== -1 || // lgtm [js/incomplete-url-substring-sanitization]
url.toLowerCase().indexOf('amp.cloudflare.com') !== -1); // lgtm [js/incomplete-url-substring-sanitization]
return url.toLowerCase().indexOf('cdn.ampproject.org'); // lgtm [js/incomplete-url-substring-sanitization]
Copy link
Copy Markdown
Contributor

@jridgewell jridgewell Feb 24, 2020

Choose a reason for hiding this comment

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

This should be !== -1

Copy link
Copy Markdown
Contributor Author

@oliy oliy Feb 26, 2020

Choose a reason for hiding this comment

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

Oops. Bad edit. Fixed.

@oliy oliy force-pushed the oli/remove-cloudflare branch from 9c1eb10 to baea34f Compare February 26, 2020 00:14
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Feb 26, 2020

This pull request fixes 1 alert when merging baea34f into 950ebea - view on LGTM.com

fixed alerts:

  • 1 for Incomplete URL substring sanitization

@Gregable
Copy link
Copy Markdown
Member

Gregable commented Mar 2, 2020

Anything left here that needs further action for submitting?

@jridgewell
Copy link
Copy Markdown
Contributor

jridgewell commented Mar 2, 2020

Just a rebase

@oliy oliy force-pushed the oli/remove-cloudflare branch from baea34f to 3667bed Compare March 4, 2020 20:01
@oliy
Copy link
Copy Markdown
Contributor Author

oliy commented Mar 4, 2020

Rebasing!

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Mar 4, 2020

This pull request fixes 1 alert when merging 3667bed into ba61489 - view on LGTM.com

fixed alerts:

  • 1 for Incomplete URL substring sanitization

jridgewell
jridgewell reviewed Mar 4, 2020
View reviewed changes
build-system/test-configs/dep-check-config.js Outdated
@@ -162,13 +162,6 @@ exports.rules = [
allowlist: [
// See todo note in ads/_a4a-config.js
'ads/_a4a-config.js->' +
Copy link
Copy Markdown
Contributor

@jridgewell jridgewell Mar 4, 2020

Choose a reason for hiding this comment

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

Ahh, need to delete this line, too.

Copy link
Copy Markdown
Contributor Author

@oliy oliy Mar 9, 2020

Choose a reason for hiding this comment

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

Thanks! Done.

@oliy oliy force-pushed the oli/remove-cloudflare branch from 3667bed to 2c746b1 Compare March 9, 2020 17:11
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Mar 9, 2020

This pull request fixes 1 alert when merging 2c746b1 into e705eea - view on LGTM.com

fixed alerts:

  • 1 for Incomplete URL substring sanitization

@oliy
Copy link
Copy Markdown
Contributor Author

oliy commented Mar 10, 2020

I don't know why the visual change happened with the last change. It shouldn't have affected things. Is this PR ready for merging?

@jridgewell
Copy link
Copy Markdown
Contributor

jridgewell commented Mar 10, 2020

Seems like a flaky visual test. I manually approved, merging.

@jridgewell jridgewell merged commit 6935ec5 into ampproject:master Mar 10, 2020
twifkak added a commit to twifkak/amphtml that referenced this pull request Mar 12, 2020
@twifkak
cl/300599497 Revision bump for ampproject#26912
679ee6f
@twifkak twifkak mentioned this pull request Mar 12, 2020
Merged
twifkak added a commit that referenced this pull request Mar 12, 2020
@twifkak @honeybadgerdontcare
Validator rollup (#27201)
527898a 
* cl/299411284 Allow links with `tel` scheme in email spec

* cl/300054759 github commit msg missing or malformed

* cl/300575599 Revision bump for #27098

* cl/300578001 Revision bump for #27132

* cl/300590811 Revision bump for #27027

* cl/300593269 Revision bump for #27170

* cl/300596115 Revision bump for #27134

* cl/300598356 Revision bump for #27076

* cl/300599497 Revision bump for #26912

Co-authored-by: honeybadgerdontcare <sedano@google.com>
This was referenced Mar 28, 2020
Closed
Merged
@calebcordry calebcordry mentioned this pull request Mar 31, 2020
Merged
@vfedoseev vfedoseev mentioned this pull request Apr 10, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jridgewell jridgewell jridgewell approved these changes

@Gregable Gregable Gregable approved these changes

Assignees

No one assigned

Labels

cla: yes PR use: In Beta / Experimental PR use: In Stable WG: caching

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@oliy @jridgewell @Gregable @kristoferbaxter @googlebot @ampprojectbot