Merge lp:~rharding/charmworld/handle-0-rev into lp:charmworld

[Richard Harding]

Update proof to handle 0 rev charms.

- When parsing the store url for a rev the regex would return the int 0
- Proof checks to determine if a revision was defined would use if revision
and fail (bool(0) == False)
- Updates the if checks to use if revision is not None, if revision is None
directly.

https://codereview.appspot.com/23600043/

R=bac

Update proof to handle 0 rev charms.

- When parsing the store url for a rev the regex would return the int 0
- Proof checks to determine if a revision was defined would use if revision
and fail (bool(0) == False)
- Updates the if checks to use if revision is not None, if revision is None
directly.

https://codereview.appspot.com/23600043/

[Richard Harding (rharding)]

Reviewers: mp+194526_code.launchpad.net,

Message:
Please take a look.

Description:
Update proof to handle 0 rev charms.

- When parsing the store url for a rev the regex would return the int 0
- Proof checks to determine if a revision was defined would use if
revision
and fail (bool(0) == False)
- Updates the if checks to use if revision is not None, if revision is
None
directly.

https://code.launchpad.net/~rharding/charmworld/handle-0-rev/+merge/194526

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/23600043/

Affected files (+8, -5 lines):
   A [revision details]
   M charmworld/models.py
   M charmworld/views/tests/test_proof.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20131107014037-izzty1sphugl28b5
+New revision: <email address hidden>

Index: charmworld/models.py
=== modified file 'charmworld/models.py'
--- charmworld/models.py 2013-11-06 02:32:43 +0000
+++ charmworld/models.py 2013-11-08 14:53:06 +0000
@@ -1604,14 +1604,14 @@
          if self.assume_store_url:
              thoughts.append('Assuming the data provided is a store url.')

- if self.revision:
+ if self.revision is not None:
              thoughts.append(
                  'Looking for specific revision: ' + str(self.revision))

          if self.store_url:
              thoughts.append(
                  'Looking for store url: ' + self.store_url)
- if not self.revision:
+ if self.revision is None:
                  thoughts.append(
                      'Regex search for store_url since no revision found: '
+
                      self.store_url)
@@ -1636,7 +1636,7 @@
          # The description includes a cs: charm store url, use that.
          # That charmstore (cs:) url may or may not have a revision in it.
If
          # not, use a regex query to find charms regardless of revision.
- if charm_description.revision:
+ if charm_description.revision is not None:
              charm_query = {'store_url': charm_description.store_url}
          else:
              # Charm names and series which make up a store_url are
garunteed
@@ -1653,7 +1653,7 @@
      elif charm_description.branch:
          # The description includes a branch_spec to search for.
          charm_query = {u'branch_spec': charm_description.branch}
- if charm_description.revision:
+ if charm_description.revision is not None:
              # Make sure we check the store revision.
              charm_query['store_data.revision'] = charm_description.revision
      elif charm_description.charm and charm_description.series:

Index: charmworld/views/tests/test_proof.py
=== modified file 'charmworld/views/tests/test_proof.py'
--- charmworld/views/tests/test_proof.py 2013-11-01 19:09:08 +0000
+++ charmworld/views/tests/test_proof.py 2013-11-08 14:53:06 +0000
@@ -15,7 +15,8 @@
          # database:mongodb
          _id, charm_one = factory.makeCharm(
              self.db,
- descripti...

[Richard Harding (rharding)]

Reviewer comments below.

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

https://codereview.appspot.com/23600043/diff/1/charmworld/models.py#newcode1607
charmworld/models.py:1607: if self.revision is not None:
if 0 would fail, so have to check for None directly.

https://codereview.appspot.com/23600043/diff/1/charmworld/views/tests/test_proof.py
File charmworld/views/tests/test_proof.py (right):

https://codereview.appspot.com/23600043/diff/1/charmworld/views/tests/test_proof.py#newcode19
charmworld/views/tests/test_proof.py:19: store_revision=0,
to test, just make sure all tests pass with one of the default charms
being at rev 0.

https://codereview.appspot.com/23600043/

[Brad Crittenden (bac)]

LGTM, no qa

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

https://codereview.appspot.com/23600043/diff/1/charmworld/models.py#newcode1607
charmworld/models.py:1607: if self.revision is not None:
Good change.

https://codereview.appspot.com/23600043/

[Juju Gui Bot (juju-gui-bot)]

review: Approve (continuous-integration)

=== modified file 'charmworld/models.py'
--- charmworld/models.py	2013-11-08 14:32:04 +0000
+++ charmworld/models.py	2013-11-08 14:58:54 +0000
@@ -1611,14 +1611,14 @@
         if self.assume_store_url:
             thoughts.append('Assuming the data provided is a store url.')
 
-        if self.revision:
+        if self.revision is not None:
             thoughts.append(
                 'Looking for specific revision: ' + str(self.revision))
 
         if self.store_url:
             thoughts.append(
                 'Looking for store url: ' + self.store_url)
-            if not self.revision:
+            if self.revision is None:
                 thoughts.append(
                     'Regex search for store_url since no revision found: ' +
                     self.store_url)
@@ -1643,7 +1643,7 @@
         # The description includes a cs: charm store url, use that.
         # That charmstore (cs:) url may or may not have a revision in it. If
         # not, use a regex query to find charms regardless of revision.
-        if charm_description.revision:
+        if charm_description.revision is not None:
             charm_query = {'store_url': charm_description.store_url}
         else:
             # Charm names and series which make up a store_url are garunteed
@@ -1660,7 +1660,7 @@
     elif charm_description.branch:
         # The description includes a branch_spec to search for.
         charm_query = {u'branch_spec': charm_description.branch}
-        if charm_description.revision:
+        if charm_description.revision is not None:
             # Make sure we check the store revision.
             charm_query['store_data.revision'] = charm_description.revision
     elif charm_description.charm and charm_description.series:

=== modified file 'charmworld/views/tests/test_proof.py'
--- charmworld/views/tests/test_proof.py	2013-11-01 19:09:08 +0000
+++ charmworld/views/tests/test_proof.py	2013-11-08 14:58:54 +0000
@@ -15,7 +15,8 @@
         # database:mongodb
         _id, charm_one = factory.makeCharm(
             self.db,
-            description=''
+            description='',
+            store_revision=0,
         )
 
         _id, charm_two = factory.makeCharm(