Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(794)

Issue 29348650: Issue 3199 - "Firefox" string is displayed on several error messages (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 4 months ago by diegocarloslima
Modified:
11 months, 2 weeks ago
Reviewers:
anton, Felix Dahlke
CC:
René Jeschke
Visibility:
Public.

Description

Issue 3199 - "Firefox" string is displayed on several error messages Sibling review for this issue: https://codereview.adblockplus.org/29341317/

Patch Set 1 #

Total comments: 2

Patch Set 2 : Removed unrelated changes and added 2 more entity exceptions #

Total comments: 10

Patch Set 3 : Some readability improvements according to Felix's suggestions #

Total comments: 1

Patch Set 4 : Fixing typo #

Unified diffs Side-by-side diffs Delta from patch set Stats (+166 lines, -32 lines) Patch
M mozharness/abb/transform_locales.py View 1 2 3 3 chunks +166 lines, -32 lines 0 comments Download

Messages

Total messages: 9
diegocarloslima
1 year, 4 months ago (2016-07-26 01:32:42 UTC) #1
René Jeschke
https://codereview.adblockplus.org/29348650/diff/29348651/mozharness/abb/transform_locales.py File mozharness/abb/transform_locales.py (right): https://codereview.adblockplus.org/29348650/diff/29348651/mozharness/abb/transform_locales.py#newcode22 mozharness/abb/transform_locales.py:22: _SHORTNAME_RE = re.compile("^<ShortName>(.*)</ShortName>$") Moving this line around is a ...
1 year, 4 months ago (2016-07-26 12:49:32 UTC) #2
anton
On 2016/07/26 12:49:32, René Jeschke wrote: > https://codereview.adblockplus.org/29348650/diff/29348651/mozharness/abb/transform_locales.py > File mozharness/abb/transform_locales.py (right): > > https://codereview.adblockplus.org/29348650/diff/29348651/mozharness/abb/transform_locales.py#newcode22 ...
1 year, 2 months ago (2016-09-30 06:52:24 UTC) #3
diegocarloslima
On 2016/09/30 06:52:24, anton wrote: > On 2016/07/26 12:49:32, René Jeschke wrote: > > > ...
1 year, 1 month ago (2016-10-21 14:28:53 UTC) #4
anton
On 2016/10/21 14:28:53, diegocarloslima wrote: > On 2016/09/30 06:52:24, anton wrote: > > On 2016/07/26 ...
1 year, 1 month ago (2016-11-02 14:06:48 UTC) #5
Felix Dahlke
https://codereview.adblockplus.org/29348650/diff/29348867/mozharness/abb/transform_locales.py File mozharness/abb/transform_locales.py (right): https://codereview.adblockplus.org/29348650/diff/29348867/mozharness/abb/transform_locales.py#newcode20 mozharness/abb/transform_locales.py:20: _VALUES_LOCALE_RE = re.compile("^values-([a-z]{2,3}(?:-r[A-Z]{2})?)$") What's the "r" for here? https://codereview.adblockplus.org/29348650/diff/29348867/mozharness/abb/transform_locales.py#newcode61 ...
1 year ago (2016-12-13 13:06:06 UTC) #6
Felix Dahlke
https://codereview.adblockplus.org/29348650/diff/29348867/mozharness/abb/transform_locales.py File mozharness/abb/transform_locales.py (right): https://codereview.adblockplus.org/29348650/diff/29348867/mozharness/abb/transform_locales.py#newcode20 mozharness/abb/transform_locales.py:20: _VALUES_LOCALE_RE = re.compile("^values-([a-z]{2,3}(?:-r[A-Z]{2})?)$") On 2016/12/13 13:06:05, Felix Dahlke wrote: ...
1 year ago (2016-12-13 13:11:15 UTC) #7
diegocarloslima
https://codereview.adblockplus.org/29348650/diff/29348867/mozharness/abb/transform_locales.py File mozharness/abb/transform_locales.py (right): https://codereview.adblockplus.org/29348650/diff/29348867/mozharness/abb/transform_locales.py#newcode61 mozharness/abb/transform_locales.py:61: _ENTITY_EXCEPTIONS = [ On 2016/12/13 13:06:05, Felix Dahlke wrote: ...
11 months, 4 weeks ago (2016-12-21 13:21:49 UTC) #8
Felix Dahlke
11 months, 2 weeks ago (2017-01-02 14:27:16 UTC) #9
LGTM, aside from a grammer nit :P

https://codereview.adblockplus.org/29348650/diff/29369542/mozharness/abb/tran...
File mozharness/abb/transform_locales.py (right):

https://codereview.adblockplus.org/29348650/diff/29369542/mozharness/abb/tran...
mozharness/abb/transform_locales.py:62: # Some string values that contains
Firefox such as 'Firefox Sync' shouldn't be
Nit: s/contains/contain/
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5