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

Issue 5655474749833216: Issue 2658 - Got rid of try..catch for filter validation (Closed)

Created:
June 7, 2015, 2:47 p.m. by Sebastian Noack
Modified:
June 15, 2015, 7:13 a.m.
Reviewers:
kzar
CC:
Wladimir Palant, Thomas Greiner
Visibility:
Public.

Description

Issue 2658 - Got rid of try..catch for filter validation

Patch Set 1 #

Total comments: 6

Patch Set 2 : Improved comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+89 lines, -91 lines) Patch
M background.js View 1 chunk +6 lines, -9 lines 0 comments Download
M lib/filterValidation.js View 1 3 chunks +61 lines, -51 lines 0 comments Download
M options.js View 2 chunks +10 lines, -18 lines 0 comments Download
M qunit/tests/filterValidation.js View 3 chunks +12 lines, -13 lines 0 comments Download

Messages

Total messages: 4
Sebastian Noack
June 7, 2015, 2:49 p.m. (2015-06-07 14:49:25 UTC) #1
kzar
Assuming it's been tested and apart from nits LGTM http://codereview.adblockplus.org/5655474749833216/diff/5629499534213120/lib/filterValidation.js File lib/filterValidation.js (right): http://codereview.adblockplus.org/5655474749833216/diff/5629499534213120/lib/filterValidation.js#newcode43 lib/filterValidation.js:43: ...
June 8, 2015, 11:28 a.m. (2015-06-08 11:28:04 UTC) #2
Sebastian Noack
http://codereview.adblockplus.org/5655474749833216/diff/5629499534213120/lib/filterValidation.js File lib/filterValidation.js (right): http://codereview.adblockplus.org/5655474749833216/diff/5629499534213120/lib/filterValidation.js#newcode43 lib/filterValidation.js:43: * @property {?Filter} [filter] The parsed filter is it ...
June 8, 2015, noon (2015-06-08 12:00:08 UTC) #3
kzar
June 8, 2015, 12:21 p.m. (2015-06-08 12:21:33 UTC) #4
LGTM

Powered by Google App Engine
This is Rietveld