[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-----
Hash: SHA1

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
| well.)
|
| 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...
|
| -Andrew.
|
|

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

BB:approve

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.)

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFH/GOxJdeBCYSNAAMRAoRKAJ9v8Q/2vYaSEOTSN2FcRrqWaan7hACgloWU
xUj1W0GyJe2HsQQ9bFfRBoU=
=Ctl3
-----END PGP SIGNATURE-----




More information about the bazaar mailing list