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

Issue 29587637: Issue 5891 - Add missing subscription information to issue reporter (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 1 month ago by Wladimir Palant
Modified:
2 years, 1 month ago
Reviewers:
kzar, Manish Jethani
CC:
Thomas Greiner
Base URL:
https://hg.adblockplus.org/adblockplusui/
Visibility:
Public.

Description

Issue 5891 - Add missing subscription information to issue reporter

Patch Set 1 #

Total comments: 5
Unified diffs Side-by-side diffs Delta from patch set Stats (+38 lines, -4 lines) Patch
M issue-reporter.js View 1 chunk +21 lines, -1 line 0 comments Download
M messageResponder.js View 3 chunks +17 lines, -3 lines 5 comments Download

Messages

Total messages: 6
Wladimir Palant
2 years, 1 month ago (2017-10-24 12:40:27 UTC) #1
kzar
https://codereview.adblockplus.org/29587637/diff/29587638/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29587637/diff/29587638/messageResponder.js#newcode413 messageResponder.js:413: result.disabledFilters = s.filters Does this pass linting? The indentation ...
2 years, 1 month ago (2017-10-24 14:14:19 UTC) #2
Wladimir Palant
https://codereview.adblockplus.org/29587637/diff/29587638/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29587637/diff/29587638/messageResponder.js#newcode413 messageResponder.js:413: result.disabledFilters = s.filters On 2017/10/24 14:14:18, kzar wrote: > ...
2 years, 1 month ago (2017-10-24 14:18:59 UTC) #3
kzar
LGTM https://codereview.adblockplus.org/29587637/diff/29587638/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29587637/diff/29587638/messageResponder.js#newcode413 messageResponder.js:413: result.disabledFilters = s.filters On 2017/10/24 14:18:59, Wladimir Palant ...
2 years, 1 month ago (2017-10-24 14:20:24 UTC) #4
Manish Jethani
https://codereview.adblockplus.org/29587637/diff/29587638/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29587637/diff/29587638/messageResponder.js#newcode413 messageResponder.js:413: result.disabledFilters = s.filters On 2017/10/24 14:18:59, Wladimir Palant wrote: ...
2 years, 1 month ago (2017-10-24 14:38:25 UTC) #5
Wladimir Palant
2 years, 1 month ago (2017-10-25 11:05:01 UTC) #6
https://codereview.adblockplus.org/29587637/diff/29587638/messageResponder.js
File messageResponder.js (right):

https://codereview.adblockplus.org/29587637/diff/29587638/messageResponder.js...
messageResponder.js:413: result.disabledFilters = s.filters
On 2017/10/24 14:38:25, Manish Jethani wrote:
> How about moving s.filter to the next line with a 2-space indent?

Not convinced that this would be better, so I better leave things as they are.
Sign in to reply to this message.

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