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

Issue 29322488: Issue 2376 - mockup causing error (Closed)

Created:
July 15, 2015, 5:26 p.m. by saroyanm
Modified:
July 16, 2015, 10:15 a.m.
Reviewers:
Thomas Greiner
Visibility:
Public.

Description

Issue 2376 - parseFilters function in mockup return wrong object

Patch Set 1 #

Total comments: 2

Patch Set 2 : Fixed 80 char limit #

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

Messages

Total messages: 6
saroyanm
Thomas can you please have a look.
July 15, 2015, 5:32 p.m. (2015-07-15 17:32:46 UTC) #1
Thomas Greiner
https://codereview.adblockplus.org/29322488/diff/29322489/background.js File background.js (right): https://codereview.adblockplus.org/29322488/diff/29322489/background.js#newcode179 background.js:179: text.split("\n").map(modules.filterClasses.Filter.fromText), errors: []}; Again, mind the 80 characters limit. ...
July 15, 2015, 5:52 p.m. (2015-07-15 17:52:51 UTC) #2
saroyanm
https://codereview.adblockplus.org/29322488/diff/29322489/background.js File background.js (right): https://codereview.adblockplus.org/29322488/diff/29322489/background.js#newcode179 background.js:179: text.split("\n").map(modules.filterClasses.Filter.fromText), errors: []}; On 2015/07/15 17:52:50, Thomas Greiner wrote: ...
July 15, 2015, 5:56 p.m. (2015-07-15 17:56:57 UTC) #3
Thomas Greiner
LGTM It would be great if you could come up with a more descriptive commit ...
July 16, 2015, 10:09 a.m. (2015-07-16 10:09:41 UTC) #4
saroyanm
On 2015/07/16 10:09:41, Thomas Greiner wrote: > LGTM > > It would be great if ...
July 16, 2015, 10:10 a.m. (2015-07-16 10:10:43 UTC) #5
Thomas Greiner
July 16, 2015, 10:12 a.m. (2015-07-16 10:12:21 UTC) #6
On 2015/07/16 10:10:43, saroyanm wrote:
> On 2015/07/16 10:09:41, Thomas Greiner wrote:
> > LGTM
> > 
> > It would be great if you could come up with a more descriptive commit
message
> > than the one in the review title.
> 
> What about the one in the description ? 
> "parseFilters function in mockup return wrong object"

That one's fine if you replace "return" with "returns"

Powered by Google App Engine
This is Rietveld