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

Issue 5217703262420992: Issue 2181 - [cms] Test server should accept default page for subdirectories as well (Closed)

Created:
March 19, 2015, 10:31 p.m. by Wladimir Palant
Modified:
March 20, 2015, 4:22 p.m.
Reviewers:
Sebastian Noack
Visibility:
Public.

Description

Issue 2181 - [cms] Test server should accept default page for subdirectories as well

Patch Set 1 #

Total comments: 3

Patch Set 2 : Ask for forgiveness #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -19 lines) Patch
M cms/bin/test_server.py View 1 chunk +9 lines, -5 lines 0 comments Download
M cms/sources.py View 1 1 chunk +28 lines, -14 lines 3 comments Download

Messages

Total messages: 7
Wladimir Palant
March 19, 2015, 10:31 p.m. (2015-03-19 22:31:09 UTC) #1
Wladimir Palant
http://codereview.adblockplus.org/5217703262420992/diff/5629499534213120/cms/sources.py File cms/sources.py (left): http://codereview.adblockplus.org/5217703262420992/diff/5629499534213120/cms/sources.py#oldcode40 cms/sources.py:40: checked_page = config.get("locale_overrides", page) Sorry about the marginally related ...
March 19, 2015, 10:33 p.m. (2015-03-19 22:33:52 UTC) #2
Sebastian Noack
http://codereview.adblockplus.org/5217703262420992/diff/5629499534213120/cms/sources.py File cms/sources.py (right): http://codereview.adblockplus.org/5217703262420992/diff/5629499534213120/cms/sources.py#newcode35 cms/sources.py:35: if config.has_option("locale_overrides", page): I'd prefer to call config.get() without ...
March 20, 2015, 9:47 a.m. (2015-03-20 09:47:40 UTC) #3
Wladimir Palant
http://codereview.adblockplus.org/5217703262420992/diff/5629499534213120/cms/sources.py File cms/sources.py (right): http://codereview.adblockplus.org/5217703262420992/diff/5629499534213120/cms/sources.py#newcode35 cms/sources.py:35: if config.has_option("locale_overrides", page): On 2015/03/20 09:47:41, Sebastian Noack wrote: ...
March 20, 2015, 3:42 p.m. (2015-03-20 15:42:36 UTC) #4
Sebastian Noack
http://codereview.adblockplus.org/5217703262420992/diff/5685265389584384/cms/sources.py File cms/sources.py (right): http://codereview.adblockplus.org/5217703262420992/diff/5685265389584384/cms/sources.py#newcode23 cms/sources.py:23: import subprocess json, subprocess and zipfile seem to be ...
March 20, 2015, 3:50 p.m. (2015-03-20 15:50:13 UTC) #5
Wladimir Palant
http://codereview.adblockplus.org/5217703262420992/diff/5685265389584384/cms/sources.py File cms/sources.py (right): http://codereview.adblockplus.org/5217703262420992/diff/5685265389584384/cms/sources.py#newcode23 cms/sources.py:23: import subprocess On 2015/03/20 15:50:13, Sebastian Noack wrote: > ...
March 20, 2015, 4:04 p.m. (2015-03-20 16:04:21 UTC) #6
Sebastian Noack
March 20, 2015, 4:11 p.m. (2015-03-20 16:11:36 UTC) #7
LGTM

http://codereview.adblockplus.org/5217703262420992/diff/5685265389584384/cms/...
File cms/sources.py (right):

http://codereview.adblockplus.org/5217703262420992/diff/5685265389584384/cms/...
cms/sources.py:23: import subprocess
On 2015/03/20 16:04:21, Wladimir Palant wrote:
> On 2015/03/20 15:50:13, Sebastian Noack wrote:
> > json, subprocess and zipfile seem to be unused. So while cleaning up imports
> > mind removing those?
> 
> I actually verified that all of them are used. Did you forget to expand?

Oh yes, I did.

Powered by Google App Engine
This is Rietveld