Code review comment for lp:~vila/bzr/config-lock-remote

Revision history for this message
John A Meinel (jameinel) wrote :

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

On 05/26/2011 08:33 AM, Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging lp:~vila/bzr/config-lock-remote into lp:bzr with lp:~vila/bzr/config-lock-branch as a prerequisite.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> For more details, see:
> https://code.launchpad.net/~vila/bzr/config-lock-remote/+merge/62418
>
> This add supports for the remote branch.conf files.
>
> It's an incremental improvement, compatible with all servers since it relies on the vfs layer only.
>
> I plan to add a smart verb only when I switch to the new lock scheme (single read/write for a given config file for a given bzr operation).
>
>
     def lock_write(self, token=None):
- - return self.branch.lock_write(token)
+ return self.branch_ref().lock_write(token)
...

     def unlock(self):
- - return self.branch.unlock()
+ return self.branch_ref().unlock()

^- If you are using a weak_ref you need to handle when branch_ref()
returns None. (because the branch has gone away)

I realize this shouldn't generally happen (I assume the branch is
holding a direct reference to the config object), but it is something
you should at least be prepared for.

Otherwise this just looks like some minor tweaking to the existing code,
which seems fine to me.

John
=:->

 review: approve

(Pending responses to Martin's questions.)

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

iEYEARECAAYFAk3d/cYACgkQJdeBCYSNAAPkhACgoTg1RLzRI/rdvsy5WZgksqJx
FLAAoJPXZA6UrekhZYDI6dNMHWwTOaxD
=a0ub
-----END PGP SIGNATURE-----

review: Approve

« Back to merge proposal