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

Issue 29713631: Issue 5760 - Use relative require paths (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 12 months ago by Jon Sonesen
Modified:
1 year, 10 months ago
Reviewers:
kzar, Sebastian Noack
Visibility:
Public.

Description

Issue 5760 - Use relative require paths

Patch Set 1 #

Total comments: 10

Patch Set 2 : Address PS1 Comments, rebase #

Total comments: 5

Patch Set 3 : Address PS2 comments, rebase #

Patch Set 4 : Address PS3 comment #

Total comments: 8

Patch Set 5 : Address PS4 comments, rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+117 lines, -107 lines) Patch
M background.js View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M composer.postload.js View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M include.preload.js View 1 2 1 chunk +3 lines, -2 lines 0 comments Download
M lib/csp.js View 1 2 3 4 1 chunk +6 lines, -5 lines 0 comments Download
M lib/cssInjection.js View 1 chunk +8 lines, -8 lines 0 comments Download
M lib/devtools.js View 1 2 3 4 1 chunk +8 lines, -7 lines 0 comments Download
M lib/filterComposer.js View 1 2 3 4 2 chunks +9 lines, -10 lines 0 comments Download
M lib/filterValidation.js View 1 chunk +1 line, -1 line 0 comments Download
M lib/firefoxDataCleanup.js View 1 1 chunk +6 lines, -5 lines 0 comments Download
M lib/icon.js View 1 chunk +1 line, -1 line 0 comments Download
M lib/messaging.js View 1 chunk +1 line, -1 line 0 comments Download
M lib/notificationHelper.js View 1 1 chunk +9 lines, -7 lines 0 comments Download
M lib/options.js View 1 2 3 4 1 chunk +3 lines, -3 lines 0 comments Download
M lib/popupBlocker.js View 1 1 chunk +6 lines, -5 lines 0 comments Download
M lib/prefs.js View 1 2 chunks +2 lines, -2 lines 0 comments Download
M lib/requestBlocker.js View 1 1 chunk +10 lines, -9 lines 0 comments Download
M lib/stats.js View 1 chunk +4 lines, -4 lines 0 comments Download
M lib/subscriptionInit.js View 1 1 chunk +11 lines, -9 lines 0 comments Download
M lib/uninstall.js View 1 chunk +3 lines, -3 lines 0 comments Download
M lib/url.js View 1 chunk +2 lines, -2 lines 0 comments Download
M lib/utils.js View 1 chunk +1 line, -1 line 0 comments Download
M lib/whitelisting.js View 1 2 3 4 1 chunk +8 lines, -8 lines 0 comments Download
M qunit/common.js View 1 2 3 4 1 chunk +6 lines, -6 lines 0 comments Download
M qunit/tests/cssEscaping.js View 1 2 3 4 1 chunk +3 lines, -2 lines 0 comments Download
M qunit/tests/filterValidation.js View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M qunit/tests/prefs.js View 1 1 chunk +1 line, -1 line 0 comments Download
M qunit/tests/url.js View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 12
Jon Sonesen
1 year, 12 months ago (2018-03-03 04:10:08 UTC) #1
Sebastian Noack
I added Dave as reviewer, since he requested this change, and is module owner peer. ...
1 year, 11 months ago (2018-03-06 21:18:17 UTC) #2
kzar
Sorry for the delay, please can you rebase this on top of the next branch? ...
1 year, 11 months ago (2018-03-19 21:50:08 UTC) #3
kzar
Also what about thes test files? Specifically: - qunit/common.js - qunit/tests/url.js - qunit/tests/prefs.js - qunit/tests/filterValidation.js ...
1 year, 11 months ago (2018-03-20 13:48:18 UTC) #4
Jon Sonesen
Thanks for looking at this, no worries on the delay there were alot of reviews ...
1 year, 11 months ago (2018-03-20 23:25:18 UTC) #5
kzar
Looks much better thanks, please could you rebase again? https://codereview.adblockplus.org/29713631/diff/29728665/include.preload.js File include.preload.js (right): https://codereview.adblockplus.org/29713631/diff/29728665/include.preload.js#newcode20 include.preload.js:20: ...
1 year, 11 months ago (2018-03-28 13:50:33 UTC) #6
Jon Sonesen
Thanks for looking at this again and catching those mistakes! https://codereview.adblockplus.org/29713631/diff/29728665/include.preload.js File include.preload.js (right): https://codereview.adblockplus.org/29713631/diff/29728665/include.preload.js#newcode20 ...
1 year, 11 months ago (2018-03-30 03:09:56 UTC) #7
kzar
https://codereview.adblockplus.org/29713631/diff/29728665/lib/filterComposer.js File lib/filterComposer.js (right): https://codereview.adblockplus.org/29713631/diff/29728665/lib/filterComposer.js#newcode290 lib/filterComposer.js:290: if (info.application == "firefox" && On 2018/03/30 03:09:55, Jon ...
1 year, 11 months ago (2018-04-03 12:08:58 UTC) #8
Jon Sonesen
On 2018/04/03 12:08:58, kzar wrote: > https://codereview.adblockplus.org/29713631/diff/29728665/lib/filterComposer.js > File lib/filterComposer.js (right): > > https://codereview.adblockplus.org/29713631/diff/29728665/lib/filterComposer.js#newcode290 > ...
1 year, 11 months ago (2018-04-03 17:16:56 UTC) #9
kzar
What about qunit/common.js? What about background.js in the root of the repository? Please could you ...
1 year, 10 months ago (2018-04-05 11:14:57 UTC) #10
Jon Sonesen
Rebase is done comments addressed, sorry for the silly little things again. Thanks for being ...
1 year, 10 months ago (2018-04-05 23:01:21 UTC) #11
kzar
1 year, 10 months ago (2018-04-06 10:25:19 UTC) #12
LGTM assuming you've tested. Please push to the next branch instead of the
master branch though!
Sign in to reply to this message.

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