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

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

[Wladimir Palant, 2015-03-19 22:33:52]

http://codereview.adblockplus.org/5217703262420992/diff/5629499534213120/cms/...
File cms/sources.py (left):

http://codereview.adblockplus.org/5217703262420992/diff/5629499534213120/cms/...
cms/sources.py:40: checked_page = config.get("locale_overrides", page)
Sorry about the marginally related changes but this logic was actually broken
after #2133 landed - locale_overrides apply to locales but they shouldn't change
what pages we are looking for.

[Sebastian Noack, 2015-03-20 09:47:40]

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

http://codereview.adblockplus.org/5217703262420992/diff/5629499534213120/cms/...
cms/sources.py:35: if config.has_option("locale_overrides", page):
I'd prefer to call config.get() without prior check catching the exception. That
way the option name must not be hard-coded twice.

[Wladimir Palant, 2015-03-20 15:42:36]

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

http://codereview.adblockplus.org/5217703262420992/diff/5629499534213120/cms/...
cms/sources.py:35: if config.has_option("locale_overrides", page):
On 2015/03/20 09:47:41, Sebastian Noack wrote:
> I'd prefer to call config.get() without prior check catching the exception.
That
> way the option name must not be hard-coded twice.

Done.

[Sebastian Noack, 2015-03-20 15:50:13]

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
json, subprocess and zipfile seem to be unused. So while cleaning up imports
mind removing those?

[Wladimir Palant, 2015-03-20 16:04:21]

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 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?

[Sebastian Noack, 2015-03-20 16:11:36]

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.