Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1348)

Issue 29829611: Issue 6786 - Display the Safari App Extension migration page (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year ago by kzar
Modified:
1 year ago
Reviewers:
Sebastian Noack
Visibility:
Public.

Description

Issue 6786 - Display the Safari App Extension migration page

Patch Set 1 #

Total comments: 7

Patch Set 2 : Addressed Sebastian's feedback #

Patch Set 3 : Only retry on failure #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -0 lines) Patch
M background.js View 1 2 1 chunk +22 lines, -0 lines 0 comments Download

Messages

Total messages: 11
kzar
Patch Set 1
1 year ago (2018-07-13 17:37:57 UTC) #1
Sebastian Noack
https://codereview.adblockplus.org/29829611/diff/29829612/background.js File background.js (right): https://codereview.adblockplus.org/29829611/diff/29829612/background.js#newcode72 background.js:72: fetch("https://eyeo.to/adblockplus/safari-app-extension-migration") I wonder, should we perhaps just send a ...
1 year ago (2018-07-13 18:01:35 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29829611/diff/29829612/safari/ext/background.js File safari/ext/background.js (right): https://codereview.adblockplus.org/29829611/diff/29829612/safari/ext/background.js#newcode598 safari/ext/background.js:598: if (page.url.href == url && tab.browserWindow == safari.application.activeBrowserWindow) I ...
1 year ago (2018-07-13 18:04:19 UTC) #3
Sebastian Noack
https://codereview.adblockplus.org/29829611/diff/29829612/safari/ext/background.js File safari/ext/background.js (right): https://codereview.adblockplus.org/29829611/diff/29829612/safari/ext/background.js#newcode598 safari/ext/background.js:598: if (page.url.href == url && tab.browserWindow == safari.application.activeBrowserWindow) On ...
1 year ago (2018-07-13 18:06:37 UTC) #4
kzar
Patch Set 2 : Addressed Sebastian's feedback https://codereview.adblockplus.org/29829611/diff/29829612/background.js File background.js (right): https://codereview.adblockplus.org/29829611/diff/29829612/background.js#newcode72 background.js:72: fetch("https://eyeo.to/adblockplus/safari-app-extension-migration") On ...
1 year ago (2018-07-13 18:44:04 UTC) #5
Sebastian Noack
The changes look good to me, if we really want to keep showing that page ...
1 year ago (2018-07-13 18:50:00 UTC) #6
kzar
On 2018/07/13 18:50:00, Sebastian Noack wrote: > The changes look good to me, if we ...
1 year ago (2018-07-13 19:02:12 UTC) #7
kzar
On 2018/07/13 18:50:00, Sebastian Noack wrote: > The changes look good to me, if we ...
1 year ago (2018-07-13 19:02:13 UTC) #8
Sebastian Noack
On 2018/07/13 19:02:13, kzar wrote: > Fair enough, do you suggest once a week or ...
1 year ago (2018-07-13 19:09:07 UTC) #9
kzar
Patch Set 3 : Only retry on failure
1 year ago (2018-07-13 19:39:09 UTC) #10
Sebastian Noack
1 year ago (2018-07-13 19:41:27 UTC) #11
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5