[MERGE] Add Branch.set_last_revision_info smart request
John Arbash Meinel
john at arbash-meinel.com
Wed Apr 9 07:35:30 BST 2008
-----BEGIN PGP SIGNED MESSAGE-----
Andrew Bennetts wrote:
| Hi all,
| This change adds a Branch.set_last_revision_info verb to the smart protocol. In
| combination with Ian's proposed post_change_branch_tip hook
| (https://lists.ubuntu.com/archives/bazaar/2008q2/040066.html) this will make it
| possible to have server-side hooks when a branch's tip changes.
| (James Henstridge's proposed set_last_revision_info hook would probably work as
| This change depends on the “Better infrastructure for dealing with 'bad request'
| responses from a smart server” patch I sent earlier today.
| The tests are large relative to the actual new production code. It can be
| fiddly hitting all the error paths...
The one piece I don't see you testing is if the client sends across a string
which isn't an integer. It will raise ValueError in the 'do_with_locked_branch'.
I suppose that will just trigger the catch-all handler. It might be nice to have
a different error, though.
Also, this brings up the question that maybe all branches should be checking
their 'set_last_revision_info' parameter. It isn't a lot of extra work to check
the repository to make sure the revision exists, and it seems like it would help
maintain consistency. Does that seem reasonable to other people?
I also wonder if you should check that 'RemoteBranch.revision_history()' also
returns the correct information after a 'set_last_revision_info()' call.
And do we have a 'get_last_revision_info()' call? I only mention it because it
gets us 1 step further from needing VFS operations, and might be a place you
could hook in logging, etc. And since you are working in this area... (So
completely not necessary for merge, just an idea.)
I think this makes it
I have some suggestions, but nothing that needs to be done before merging this.
(Or maybe it should be :tweak depending on the other patches to be merged.)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----
More information about the bazaar