[attn aaron] differences between RevisionTree and WorkingTree

Aaron Bentley aaron.bentley at utoronto.ca
Fri Jul 28 14:01:08 BST 2006


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> On Wed, 2006-07-26 at 15:03 -0500, John Arbash Meinel wrote:
> 
>>-----BEGIN PGP SIGNED MESSAGE-----
>>Hash: SHA1
>>
>>Robert Collins wrote:
>>
>>
>>>So heres a patch that does this: changes iter_entries to yield the root.
>>>
>>>I hope this will be uncontentious - its a small change, it hasn't broken
>>>any plugins AFAICT. 
>>>
>>>The tree implementation tests I have planned at this point are primarily
>>>for ensuring test trees are available, but there are a number of tests
>>>we can move into there should someone be interested.
>>>
>>>Cheers,
>>>Rob
>>>
>>>
> 
> ...
> 
> Aaron, what do you think of this patch?

I would really like to understand the test adaption system better.

I would do this differently:

- --- bzrlib/commit.py
+++ bzrlib/commit.py
@@ -502,10 +502,11 @@
         # in bugs like #46635.  Any reason not to use/enhance
compare_trees?
         # ADHB 11-07-2006
         mutter("Selecting files for commit with filter %s",
self.specific_files)
- -        # iter_entries does not visit the ROOT_ID node so we need to call
- -        # self._emit_progress_update once by hand.
+        # at this point we dont copy the root entry:
+        entries = self.work_inv.iter_entries()
+        entries.next()
         self._emit_progress_update()
- -        for path, new_ie in self.work_inv.iter_entries():
+        for path, new_ie in entries:
             self._emit_progress_update()
             file_id = new_ie.file_id
             mutter('check %s {%s}', path, file_id)

Instead of calling entries.next() once, I'd do
        for path, new_ie in self.work_inv.iter_entries():
            self._emit_progress_update()
            if new_id is self.work_inv.root:
                continue
            file_id = new_ie.file_id
            mutter('check %s {%s}', path, file_id)

Which seems to telegraph the reasoning better.  Mostly a personal style
thing, but I think it removes the need for a comment, and therefore
removes the need to keep a comment up to date.  This pattern applies to
a lot of the code.

- -        self.assertEqual(self.sorted_ids(btree), ['a', 'b', 'c', 'd'])
+        self.assertEqual(self.sorted_ids(btree),
+            [inventory.ROOT_ID, 'a', 'b', 'c', 'd'])

I'm of two minds about introducing new uses of ROOT_ID; it's contrary to
what nested-trees does, but it is easy to search for in order to fix.

The alternative would be:
        self.assertEqual(self.sorted_ids(btree),
            [btree.inventory.root.file_id, 'a', 'b', 'c', 'd'])
Which is a weaker test, but future-proof.

So I'll take it either way, I guess.

Aaron


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFEygqU0F+nu1YWqI0RApB7AJ4y0PCv4663E1H7+ZrilX47+h6djwCfQsEV
34jStDLyJDUtoHv0AJ8Jd8w=
=9WuK
-----END PGP SIGNATURE-----




More information about the bazaar mailing list