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

Issue 5731153147330560: Noissue - Add code module for Adblock Browser for Android (Closed)

Created:
May 20, 2015, 3:26 p.m. by Felix Dahlke
Modified:
May 22, 2015, 9:34 a.m.
Reviewers:
René Jeschke, saroyanm
CC:
Wladimir Palant
Visibility:
Public.

Description

Noissue - Add code module for Adblock Browser for Android

Patch Set 1 #

Patch Set 2 : Use hyphens in string IDs #

Patch Set 3 : Rebased #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -0 lines) Patch
M pages/modules.html View 1 1 chunk +25 lines, -0 lines 2 comments Download

Messages

Total messages: 5
Felix Dahlke
May 20, 2015, 3:27 p.m. (2015-05-20 15:27:58 UTC) #1
René Jeschke
LGTM
May 20, 2015, 3:37 p.m. (2015-05-20 15:37:13 UTC) #2
Felix Dahlke
Realised Manvel should be the reviewer, being Websites module owner.
May 21, 2015, 9:12 a.m. (2015-05-21 09:12:35 UTC) #3
saroyanm
http://codereview.adblockplus.org/5731153147330560/diff/5717271485874176/pages/modules.html File pages/modules.html (right): http://codereview.adblockplus.org/5731153147330560/diff/5717271485874176/pages/modules.html#newcode290 pages/modules.html:290: <th>{{adblock-browser-for-android-owner Owner}}</th> Would be great to have same translatable ...
May 21, 2015, 9:37 a.m. (2015-05-21 09:37:32 UTC) #4
Felix Dahlke
May 21, 2015, 11:30 a.m. (2015-05-21 11:30:03 UTC) #5
Pushed: https://hg.adblockplus.org/web.adblockplus.org/rev/bdacc808f96a

http://codereview.adblockplus.org/5731153147330560/diff/5717271485874176/page...
File pages/modules.html (right):

http://codereview.adblockplus.org/5731153147330560/diff/5717271485874176/page...
pages/modules.html:290: <th>{{adblock-browser-for-android-owner Owner}}</th>
On 2015/05/21 09:37:32, saroyanm wrote:
> Would be great to have same translatable strings for: "Name", "Owner", "Super
> reviewer", "Source path(s)" for all consistent strings in this page, in that
> case no need of use "adblock-browser-for-android" prefix, because it can lead
to
> problem with translation in future, anyway if you consider it as separate
issue
> I'll quickly fix that and give you LGTM for this one, while this is side
effect
> of migration script. Hope you won't mind to review the changes in that case.

Yes we should definitely do that, but I thought it's a separate issue, yeah :)
I'll push this then.

Powered by Google App Engine
This is Rietveld