Code review comment for lp:~rharding/charmworld/fix-revisionless-store_url

Revision history for this message
Benji York (benji) wrote :

LGTM once the comments have been considered.

https://codereview.appspot.com/19990043/diff/1/charmworld/models.py
File charmworld/models.py (right):

https://codereview.appspot.com/19990043/diff/1/charmworld/models.py#newcode1630
charmworld/models.py:1630: # That cs url may or may not have a revision
in it. If not, use a
It took me a second to figure out what "cs" was. That suggests
expanding it to "charmstore" would help a less-involved reader.

https://codereview.appspot.com/19990043/diff/1/charmworld/models.py#newcode1637
charmworld/models.py:1637: "$regex": charm_description.store_url
Do we know that the regex query will return the latest version?

Is there something like a $prefix operator that we could use instead.
If not, we should meditate upon whether or not a store URL could be
interpreted as a regex. For example, if a dot were allowed in the name
it would match any character, potentially returning the wrong data.

Once that meditation has been done (or if it has already been done), a
comment noting the potential complications and that they have been
considered would be good.

https://codereview.appspot.com/19990043/

« Back to merge proposal