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

Issue 29877558: Noissue - Avoid redundant lookup in keyword-by-filter map (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 2 days ago by Manish Jethani
Modified:
27 minutes ago
Reviewers:
hub, Jon Sonesen
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Noissue - Avoid redundant lookup in keyword-by-filter map

Patch Set 1 #

Total comments: 7

Patch Set 2 : Use string comparison #

Total comments: 1

Patch Set 3 : Revert to Patch Set 1 #

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

Messages

Total messages: 17
Manish Jethani
1 week, 2 days ago (2018-09-10 12:21:53 UTC) #1
Manish Jethani
Patch Set 1 There's no need to call Map.has() when we're going to call Map.get() ...
1 week, 2 days ago (2018-09-10 12:24:27 UTC) #2
hub
https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js File lib/matcher.js (right): https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js#newcode351 lib/matcher.js:351: return !keyword; Why the `!keyword`. I'd just return the ...
6 days, 3 hours ago (2018-09-13 15:52:40 UTC) #3
Manish Jethani
https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js File lib/matcher.js (right): https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js#newcode351 lib/matcher.js:351: return !keyword; On 2018/09/13 15:52:40, hub wrote: > Why ...
5 days, 13 hours ago (2018-09-14 05:28:32 UTC) #4
hub
https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js File lib/matcher.js (right): https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js#newcode351 lib/matcher.js:351: return !keyword; On 2018/09/14 05:28:32, Manish Jethani wrote: > ...
5 days, 6 hours ago (2018-09-14 12:20:49 UTC) #5
Manish Jethani
https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js File lib/matcher.js (right): https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js#newcode351 lib/matcher.js:351: return !keyword; On 2018/09/14 12:20:49, hub wrote: > On ...
5 days, 5 hours ago (2018-09-14 13:36:25 UTC) #6
Jon Sonesen
https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js File lib/matcher.js (right): https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js#newcode351 lib/matcher.js:351: return !keyword; On 2018/09/14 13:36:25, Manish Jethani wrote: > ...
2 days ago (2018-09-17 19:13:18 UTC) #7
Manish Jethani
https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js File lib/matcher.js (right): https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js#newcode351 lib/matcher.js:351: return !keyword; On 2018/09/17 19:13:18, Jon Sonesen wrote: > ...
1 day, 19 hours ago (2018-09-17 23:36:35 UTC) #8
Jon Sonesen
On 2018/09/17 23:36:35, Manish Jethani wrote: > https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js > File lib/matcher.js (right): > > https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js#newcode351 ...
1 day, 12 hours ago (2018-09-18 07:14:26 UTC) #9
hub
https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js File lib/matcher.js (right): https://codereview.adblockplus.org/29877558/diff/29877559/lib/matcher.js#newcode351 lib/matcher.js:351: return !keyword; On 2018/09/17 23:36:35, Manish Jethani wrote: > ...
1 day, 6 hours ago (2018-09-18 12:19:40 UTC) #10
Manish Jethani
Patch Set 2: Use string comparison OK, using string comparison now. It shouldn't make a ...
1 day, 6 hours ago (2018-09-18 12:39:15 UTC) #11
Manish Jethani
https://codereview.adblockplus.org/29877558/diff/29884560/lib/matcher.js File lib/matcher.js (right): https://codereview.adblockplus.org/29877558/diff/29884560/lib/matcher.js#newcode351 lib/matcher.js:351: return keyword == ""; Well hold on, if we ...
1 day, 5 hours ago (2018-09-18 14:10:26 UTC) #12
Jon Sonesen
On 2018/09/18 14:10:26, Manish Jethani wrote: > https://codereview.adblockplus.org/29877558/diff/29884560/lib/matcher.js > File lib/matcher.js (right): > > https://codereview.adblockplus.org/29877558/diff/29884560/lib/matcher.js#newcode351 ...
3 hours, 2 minutes ago (2018-09-19 16:12:20 UTC) #13
Jon Sonesen
On 2018/09/18 14:10:26, Manish Jethani wrote: > https://codereview.adblockplus.org/29877558/diff/29884560/lib/matcher.js > File lib/matcher.js (right): > > https://codereview.adblockplus.org/29877558/diff/29884560/lib/matcher.js#newcode351 ...
3 hours, 2 minutes ago (2018-09-19 16:12:21 UTC) #14
Jon Sonesen
On 2018/09/19 16:12:21, Jon Sonesen wrote: > On 2018/09/18 14:10:26, Manish Jethani wrote: > > ...
3 hours, 1 minute ago (2018-09-19 16:13:22 UTC) #15
Manish Jethani
Jon, can you give a quick LGTM for formality sake? I've reverted to PS1.
1 hour, 12 minutes ago (2018-09-19 18:02:23 UTC) #16
Jon Sonesen
38 minutes ago (2018-09-19 18:36:49 UTC) #17
On 2018/09/19 18:02:23, Manish Jethani wrote:
> Jon, can you give a quick LGTM for formality sake? I've reverted to PS1.

Sure! LGTM
Sign in to reply to this message.

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