LGTM
https://codereview.appspot.com/21370044/diff/20001/charmworld/routes.py File charmworld/routes.py (right):
https://codereview.appspot.com/21370044/diff/20001/charmworld/routes.py#newcode57 charmworld/routes.py:57: "/bundle/~{owner}/{basket}/{rev}/{bundle}/json") On 2013/11/04 21:46:34, rharding wrote: > this could probably go into some sort of /bundle/*bits but leaving as simple > case of moving the url for the moment.
If I'm understanding you correctly, I think I like it better this way. It is easier to see the pattern matching spelled out in routes rather than in code.
https://codereview.appspot.com/21370044/diff/20001/charmworld/views/api/__init__.py File charmworld/views/api/__init__.py (right):
https://codereview.appspot.com/21370044/diff/20001/charmworld/views/api/__init__.py#newcode429 charmworld/views/api/__init__.py:429: 'personal-bundle-json-revision', **route_info) Since route_info isn't used subsequently, I don't see why it is created instead of just passing its contents as keyword arguments.
https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/test_api.py File charmworld/views/tests/test_api.py (right):
https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/test_api.py#newcode1 charmworld/views/tests/test_api.py:1: # Copyright<enter change description here if sensible> 2012, 2013 Canonical Something bad happened here.
https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/test_api.py#newcode841 charmworld/views/tests/test_api.py:841: u'http://example.com/bundle/%7Ebac/bat/4/byobu/json', On 2013/11/04 21:46:34, rharding wrote: > I'm so sorry, but the linter...he hates me.
Eww. That is bad.
It wouldn't accept the value being indented four more spaces?
https://codereview.appspot.com/21370044/diff/20001/charmworld/views/tests/test_api.py#newcode1585 charmworld/views/tests/test_api.py:1585: class TestAPI3(TestAPI, API3Mixin): On 2013/11/04 21:46:34, rharding wrote: > remove the stupid Z. Just skipping the bad test for now. See later files.
I like the explicitness of the new approach better.
https://codereview.appspot.com/21370044/
« Back to merge proposal
LGTM
https:/ /codereview. appspot. com/21370044/ diff/20001/ charmworld/ routes. py routes. py (right):
File charmworld/
https:/ /codereview. appspot. com/21370044/ diff/20001/ charmworld/ routes. py#newcode57 routes. py:57: ~{owner} /{basket} /{rev}/ {bundle} /json")
charmworld/
"/bundle/
On 2013/11/04 21:46:34, rharding wrote:
> this could probably go into some sort of /bundle/*bits but leaving as
simple
> case of moving the url for the moment.
If I'm understanding you correctly, I think I like it better this way.
It is easier to see the pattern matching spelled out in routes rather
than in code.
https:/ /codereview. appspot. com/21370044/ diff/20001/ charmworld/ views/api/ __init_ _.py views/api/ __init_ _.py (right):
File charmworld/
https:/ /codereview. appspot. com/21370044/ diff/20001/ charmworld/ views/api/ __init_ _.py#newcode429 views/api/ __init_ _.py:429: 'personal- bundle- json-revision' ,
charmworld/
**route_info)
Since route_info isn't used subsequently, I don't see why it is created
instead of just passing its contents as keyword arguments.
https:/ /codereview. appspot. com/21370044/ diff/20001/ charmworld/ views/tests/ test_api. py views/tests/ test_api. py (right):
File charmworld/
https:/ /codereview. appspot. com/21370044/ diff/20001/ charmworld/ views/tests/ test_api. py#newcode1 views/tests/ test_api. py:1: # Copyright<enter change
charmworld/
description here if sensible> 2012, 2013 Canonical
Something bad happened here.
https:/ /codereview. appspot. com/21370044/ diff/20001/ charmworld/ views/tests/ test_api. py#newcode841 views/tests/ test_api. py:841: example. com/bundle/ %7Ebac/ bat/4/byobu/ json',
charmworld/
u'http://
On 2013/11/04 21:46:34, rharding wrote:
> I'm so sorry, but the linter...he hates me.
Eww. That is bad.
It wouldn't accept the value being indented four more spaces?
https:/ /codereview. appspot. com/21370044/ diff/20001/ charmworld/ views/tests/ test_api. py#newcode1585 views/tests/ test_api. py:1585: class TestAPI3(TestAPI,
charmworld/
API3Mixin):
On 2013/11/04 21:46:34, rharding wrote:
> remove the stupid Z. Just skipping the bad test for now. See later
files.
I like the explicitness of the new approach better.
https:/ /codereview. appspot. com/21370044/