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

Issue 29469568: Issue 5331 - Adds has_string() global (Closed)

Created:
June 20, 2017, 2:51 p.m. by Jon Sonesen
Modified:
June 23, 2017, 9:19 a.m.
Reviewers:
Vasily Kuznetsov
CC:
juliandoucette
Visibility:
Public.

Description

Looks like this may be the "dumb" solution but perhaps it is not so bad considering it requires no additional changes to the code base, lowering a chance of regression. Open to changing though

Patch Set 1 #

Total comments: 8

Patch Set 2 : add protected _get_localdata method and imprve tests #

Patch Set 3 : remove de/global.json change string for en/global.json #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -4 lines) Patch
M cms/converters.py View 1 3 chunks +12 lines, -2 lines 0 comments Download
M tests/expected_output/global View 1 2 1 chunk +1 line, -1 line 0 comments Download
M tests/test_site/locales/de/translate.json View 1 1 chunk +1 line, -1 line 0 comments Download
A tests/test_site/locales/en/global.json View 1 2 1 chunk +6 lines, -0 lines 0 comments Download
M tests/test_site/pages/global.tmpl View 1 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 6
Jon Sonesen
June 20, 2017, 2:51 p.m. (2017-06-20 14:51:42 UTC) #1
Vasily Kuznetsov
Pretty cool! It turned out to be simpler than I thought. Just have a few ...
June 20, 2017, 5:38 p.m. (2017-06-20 17:38:28 UTC) #2
Jon Sonesen
https://codereview.adblockplus.org/29469568/diff/29469569/cms/converters.py File cms/converters.py (right): https://codereview.adblockplus.org/29469568/diff/29469569/cms/converters.py#newcode450 cms/converters.py:450: localedata = self._params['source'].read_locale(self._params['locale'], On 2017/06/20 17:38:27, Vasily Kuznetsov wrote: ...
June 21, 2017, 7:53 a.m. (2017-06-21 07:53:54 UTC) #3
Vasily Kuznetsov
https://codereview.adblockplus.org/29469568/diff/29469569/cms/converters.py File cms/converters.py (right): https://codereview.adblockplus.org/29469568/diff/29469569/cms/converters.py#newcode450 cms/converters.py:450: localedata = self._params['source'].read_locale(self._params['locale'], On 2017/06/21 07:53:54, Jon Sonesen wrote: ...
June 21, 2017, 8:17 a.m. (2017-06-21 08:17:00 UTC) #4
Jon Sonesen
On 2017/06/21 08:17:00, Vasily Kuznetsov wrote: > https://codereview.adblockplus.org/29469568/diff/29469569/cms/converters.py > File cms/converters.py (right): > > https://codereview.adblockplus.org/29469568/diff/29469569/cms/converters.py#newcode450 ...
June 21, 2017, 9:38 a.m. (2017-06-21 09:38:48 UTC) #5
Vasily Kuznetsov
June 21, 2017, 10:29 a.m. (2017-06-21 10:29:04 UTC) #6
LGTM

Powered by Google App Engine
This is Rietveld