Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 5438786527821824: Issue 527 - Only process notification requests sent by our own extensions (Closed)

Created:
May 22, 2014, 7:20 a.m. by Sebastian Noack
Modified:
May 22, 2014, 8:44 a.m.
Visibility:
Public.

Description

Issue 527 - Only process notification requests sent by our own extensions

Patch Set 1 #

Total comments: 3

Patch Set 2 : Addressed comments #

Total comments: 2

Patch Set 3 : Added test case #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -8 lines) Patch
M sitescripts/stats/bin/logprocessor.py View 1 2 1 chunk +11 lines, -8 lines 0 comments Download
M sitescripts/stats/test/logprocessor.py View 1 2 1 chunk +12 lines, -0 lines 0 comments Download

Messages

Total messages: 9
Sebastian Noack
May 22, 2014, 7:21 a.m. (2014-05-22 07:21:13 UTC) #1
Felix Dahlke
http://codereview.adblockplus.org/5438786527821824/diff/5629499534213120/sitescripts/stats/bin/logprocessor.py File sitescripts/stats/bin/logprocessor.py (right): http://codereview.adblockplus.org/5438786527821824/diff/5629499534213120/sitescripts/stats/bin/logprocessor.py#newcode401 sitescripts/stats/bin/logprocessor.py:401: if filename == "notification.json" and info["addonName"] not in ("adblockplus", ...
May 22, 2014, 7:35 a.m. (2014-05-22 07:35:21 UTC) #2
Wladimir Palant
http://codereview.adblockplus.org/5438786527821824/diff/5629499534213120/sitescripts/stats/bin/logprocessor.py File sitescripts/stats/bin/logprocessor.py (right): http://codereview.adblockplus.org/5438786527821824/diff/5629499534213120/sitescripts/stats/bin/logprocessor.py#newcode401 sitescripts/stats/bin/logprocessor.py:401: if filename == "notification.json" and info["addonName"] not in ("adblockplus", ...
May 22, 2014, 7:44 a.m. (2014-05-22 07:44:05 UTC) #3
Wladimir Palant
LGTM
May 22, 2014, 8:23 a.m. (2014-05-22 08:23:33 UTC) #4
Felix Dahlke
LGTM, with the whitespace change addressed. http://codereview.adblockplus.org/5438786527821824/diff/5639274879778816/sitescripts/stats/bin/logprocessor.py File sitescripts/stats/bin/logprocessor.py (right): http://codereview.adblockplus.org/5438786527821824/diff/5639274879778816/sitescripts/stats/bin/logprocessor.py#newcode431 sitescripts/stats/bin/logprocessor.py:431: Unrelated whitespace change?
May 22, 2014, 8:30 a.m. (2014-05-22 08:30:37 UTC) #5
Felix Dahlke
LGTM, with the whitespace change addressed.
May 22, 2014, 8:30 a.m. (2014-05-22 08:30:39 UTC) #6
Sebastian Noack
http://codereview.adblockplus.org/5438786527821824/diff/5629499534213120/sitescripts/stats/bin/logprocessor.py File sitescripts/stats/bin/logprocessor.py (right): http://codereview.adblockplus.org/5438786527821824/diff/5629499534213120/sitescripts/stats/bin/logprocessor.py#newcode401 sitescripts/stats/bin/logprocessor.py:401: if filename == "notification.json" and info["addonName"] not in ("adblockplus", ...
May 22, 2014, 8:40 a.m. (2014-05-22 08:40:39 UTC) #7
Felix Dahlke
LGTM!
May 22, 2014, 8:41 a.m. (2014-05-22 08:41:31 UTC) #8
Wladimir Palant
May 22, 2014, 8:42 a.m. (2014-05-22 08:42:43 UTC) #9
Even more LGTM.

Powered by Google App Engine
This is Rietveld