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

Issue 5406295150559232: wrong icons parameter value in manifest.json for chrome buildtool fix. (Closed)

Created:
March 18, 2014, 4:03 p.m. by saroyanm
Modified:
March 20, 2014, 7:04 a.m.
Reviewers:
Wladimir Palant
CC:
Sebastian Noack, Thomas Greiner
Visibility:
Public.

Description

This fix is related to current ticket: https://issues.adblockplus.org/ticket/156 Relative review: http://codereview.adblockplus.org/5676337998069760

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use PIL to determine image size #

Total comments: 4

Patch Set 3 : implement PIL in packagerChrome.py #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -15 lines) Patch
M packagerChrome.py View 1 2 4 chunks +10 lines, -15 lines 1 comment Download

Messages

Total messages: 7
saroyanm
@Wladimir Can you please have a look on this review when you will have time, ...
March 18, 2014, 4:19 p.m. (2014-03-18 16:19:41 UTC) #1
Wladimir Palant
http://codereview.adblockplus.org/5406295150559232/diff/5629499534213120/packagerChrome.py File packagerChrome.py (right): http://codereview.adblockplus.org/5406295150559232/diff/5629499534213120/packagerChrome.py#newcode77 packagerChrome.py:77: match = re.search("\d+(?=\.)", icon) Determining icon size that way ...
March 18, 2014, 5:22 p.m. (2014-03-18 17:22:50 UTC) #2
saroyanm
New patch uploaded. http://codereview.adblockplus.org/5406295150559232/diff/5629499534213120/packagerChrome.py File packagerChrome.py (right): http://codereview.adblockplus.org/5406295150559232/diff/5629499534213120/packagerChrome.py#newcode77 packagerChrome.py:77: match = re.search("\d+(?=\.)", icon) On 2014/03/18 ...
March 19, 2014, 9:04 a.m. (2014-03-19 09:04:35 UTC) #3
Wladimir Palant
http://codereview.adblockplus.org/5406295150559232/diff/5685265389584384/imageConversion.py File imageConversion.py (right): http://codereview.adblockplus.org/5406295150559232/diff/5685265389584384/imageConversion.py#newcode138 imageConversion.py:138: return Image.open(StringIO(file)).size I didn't mean to actually put this ...
March 19, 2014, 12:11 p.m. (2014-03-19 12:11:48 UTC) #4
saroyanm
Thanks for your notes Wladimir, Please have a look on last uploaded patch set. http://codereview.adblockplus.org/5406295150559232/diff/5685265389584384/imageConversion.py ...
March 19, 2014, 2:54 p.m. (2014-03-19 14:54:18 UTC) #5
Wladimir Palant
LGTM with the remaining issue addressed. http://codereview.adblockplus.org/5406295150559232/diff/5076324926357504/packagerChrome.py File packagerChrome.py (right): http://codereview.adblockplus.org/5406295150559232/diff/5076324926357504/packagerChrome.py#newcode80 packagerChrome.py:80: print 'Warning: %s ...
March 19, 2014, 9:23 p.m. (2014-03-19 21:23:06 UTC) #6
saroyanm
March 20, 2014, 7:03 a.m. (2014-03-20 07:03:55 UTC) #7
On 2014/03/19 21:23:06, Wladimir Palant wrote:
> LGTM with the remaining issue addressed.
> 
>
http://codereview.adblockplus.org/5406295150559232/diff/5076324926357504/pack...
> File packagerChrome.py (right):
> 
>
http://codereview.adblockplus.org/5406295150559232/diff/5076324926357504/pack...
> packagerChrome.py:80: print 'Warning: %s size is %ix%i, icon should be square'
%
> (icon, width, height)
> Errors and warnings should go to stderr:
> 
>   print >>sys.stderr, ...

Changes pushed,
Thanks.

Powered by Google App Engine
This is Rietveld