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

Issue 29608569: Issue 6037 - Allow popup icons without popup (Closed)

Created:
Nov. 14, 2017, 3:41 p.m. by Manish Jethani
Modified:
Nov. 17, 2017, 3:05 p.m.
Reviewers:
Sebastian Noack, tlucas
CC:
kzar
Base URL:
https://hg.adblockplus.org/buildtools/
Visibility:
Public.

Description

Issue 6037 - Allow popup icons without popup

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use endswith #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -5 lines) Patch
M packagerChrome.py View 1 1 chunk +10 lines, -5 lines 0 comments Download

Messages

Total messages: 8
Manish Jethani
Nov. 14, 2017, 3:41 p.m. (2017-11-14 15:41:17 UTC) #1
Manish Jethani
Patch Set 1 See https://codereview.adblockplus.org/29597555/ for background, we need to edit metadata.gecko to remove the ...
Nov. 14, 2017, 3:44 p.m. (2017-11-14 15:44:23 UTC) #2
tlucas
Hey Manish, just one nit. Could you also please add the review URL to #6037 ...
Nov. 14, 2017, 4:10 p.m. (2017-11-14 16:10:22 UTC) #3
Manish Jethani
Patch Set 2: Use endswith https://codereview.adblockplus.org/29608569/diff/29608570/packagerChrome.py File packagerChrome.py (right): https://codereview.adblockplus.org/29608569/diff/29608570/packagerChrome.py#newcode89 packagerChrome.py:89: elif re.search('\.html$', icons[-1]): On ...
Nov. 14, 2017, 6:40 p.m. (2017-11-14 18:40:10 UTC) #4
Manish Jethani
On 2017/11/14 16:10:22, tlucas wrote: > Could you also please add the review URL to ...
Nov. 14, 2017, 6:40 p.m. (2017-11-14 18:40:24 UTC) #5
tlucas
LGTM
Nov. 15, 2017, 8:56 a.m. (2017-11-15 08:56:10 UTC) #6
tlucas
I guess this review can be close now? https://hg.adblockplus.org/buildtools/rev/1c798cc8b402
Nov. 17, 2017, 1:33 p.m. (2017-11-17 13:33:49 UTC) #7
Manish Jethani
Nov. 17, 2017, 3:05 p.m. (2017-11-17 15:05:18 UTC) #8
Message was sent while issue was closed.
On 2017/11/17 13:33:49, tlucas wrote:
> I guess this review can be close now?
> 
> https://hg.adblockplus.org/buildtools/rev/1c798cc8b402

Yes, closed now.

Powered by Google App Engine
This is Rietveld