[MERGE] using NULL_REVISION everywhere

[Robert Collins]

sorry about thread breaking, managed to delete the mail somehow.

It might be nice to have ensure_null() issue a deprecation warning when
it sees None. (In future, after the api break). 

Generally I'm happy with this, but there are some quirks I'd like to
discuss before bumping it to +1:


@ -1779,12 +1782,12 @@
         # last_rev is not in the other_last_rev history, AND
         # other_last_rev is not in our history, and do it without pulling
         # history around
-        last_rev = self.last_revision()
-        if last_rev is not None:
+        last_rev = _mod_revision.ensure_null(self.last_revision())
+        if not _mod_revision.is_null(last_rev):

isn't this better as
         if not last_rev == NULL_REVISION
given that you just cast it to NULL on the line above.

             other.lock_read()
             try:
                 other_last_rev = other.last_revision()
-                if other_last_rev is not None:
+                if not _mod_revision.is_null(other_last_rev):
                     # neither branch is new, we have to do some work to
                     # ascertain diversion.
                     remote_graph = other.repository.get_revision_graph(


=== modified file bzrlib/repository.py // last-changed:abentley at panoramicfeedba
... ck.com-20070710211854-93ofaa70day2p8d3
--- bzrlib/repository.py
+++ bzrlib/repository.py
@@ -498,6 +498,9 @@
         for revision in revisions:
             required_trees.add(revision.revision_id)
             required_trees.update(revision.parent_ids[:1])
+        if ('null:' in required_trees):
+            import pdb; pdb.set_trace()
+

I don't think this hunk should be merged :).

=== modified file bzrlib/tests/__init__.py // last-changed:abentley at panoramicfe
... edback.com-20070711194451-3jqhye1nnd02a9uv
--- bzrlib/tests/__init__.py
+++ bzrlib/tests/__init__.py
@@ -100,7 +100,7 @@
 # Mark this python module as being part of the implementation
 # of unittest: this gives us better tracebacks where the last
 # shown frame is the test code, not our assertXYZ.
-__unittest = 1
+#__unittest = 1
 
 default_transport = LocalURLServer
 
Or this one. (BTW - why did you need to disable it ?)

=== modified file bzrlib/tests/workingtree_implementations/test_commit.py // la
...
@@ -152,7 +160,8 @@
             return
         tree.commit('foo', rev_id='foo', local=True)
         self.failIf(master.repository.has_revision('foo'))
-        self.assertEqual(None, master.last_revision())
+        self.assertTrue(_mod_revision.is_null(_mod_revision.ensure_null(
+                        master.last_revision())))

For debugging I really prefer assertEqual over assertTrue in these places. e.g.
         self.assertEqual(NULL_REVISION,
             _mod_revision.ensure_null(master.last_revision())))
 
This also applies to the change made to workingtree_implementations/test_parents.py

...
+class TestUncommit(TestCaseWithWorkingTree):
+
+    def test_uncommit_to_null(self):
+        tree = self.make_branch_and_tree('branch')
+        tree.lock_write()
+        revid = tree.commit('a revision')
+        tree.unlock()
+        uncommit.uncommit(tree.branch, tree=tree)
+        self.assertEqual([], tree.get_parent_ids())

This should go in bzrlib.tests.workingtree_implementations.test_uncommit IMO.

=== modified file bzrlib/workingtree.py // last-changed:abentley at panoramicfeedb
...
@@ -502,8 +503,8 @@
+        last_rev = _mod_revision.ensure_null(self._last_revision())
+        if _mod_revision.is_null(last_rev):

wouldn't NULL_REVISION == last_rev be better here now that you have cast
it to null? Otherwise why bother casting it?
...

@@ -2717,7 +2723,7 @@
             # only set an explicit root id if there is one to set.
             if basis_tree.inventory.root is not None:
                 wt.set_root_id(basis_tree.inventory.root.file_id)
-            if revision_id == NULL_REVISION:
+            if revision_id is None or revision_id == NULL_REVISION:

Wouldn't a call to 'is_null' be appropriate here, or use ensure_null ?

-Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070712/4ddabbb4/attachment.pgp