Code review comment for lp:~oddbloke/bzr/183559-switch-r

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

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

Martin Pool wrote:
> Martin Pool has proposed merging lp:~daniel-thewatkins/bzr/183559-switch-r into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> #183559 bzr switch should have a -r option
> https://bugs.launchpad.net/bugs/183559
>
>
> Fix for https://bugs.edge.launchpad.net/bzr/+bug/183559 adding "switch -r".
>
> I haven't read it yet but it would be good for someone to review it and if necessary pick it up.
>

1) Lots of conflicts. Looks like it was done back in 1.12 or so. A lot
of stuff doesn't line up. Probably mostly shallow

2)

=== modified file 'bzrlib/builtins.py'
- --- bzrlib/builtins.py 2009-12-23 05:42:33 +0000
+++ bzrlib/builtins.py 2010-01-05 03:39:25 +0000
@@ -5493,22 +5493,42 @@
     that of the master.
     """

- - takes_args = ['to_location']
+ takes_args = ['to_location?']
     takes_options = [Option('force',
+<<<<<<< TREE
                         help='Switch even if local commits will be lost.'),
                      Option('create-branch', short_name='b',
                         help='Create the target branch from this one
before'
                              ' switching to it.'),
+=======
+ help='Switch even if local commits will be lost.'),
+ 'revision'
+>>>>>>> MERGE-SOURCE
                      ]

^- This looks like he made it so "bzr switch -r XXX" changes the local
tree to a different rev. Which sounds like overlap with the new "bzr
update -r" functionality.

I would probably pick one and stick with it, rather than having 2 ways
to do it.

TBH switch comes to mind faster than update, though I think update fits
SVN usage better.

The rest of the code just passing down a revision_id to switch, and
having it use that RevisionTree rather than just the
to_branch.basis_tree() seems fine.

 review: needs_fixing

I'm not setting this to WIP yet, as maybe the patch-pilot wants to pick
it up?

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

iEYEARECAAYFAktCuiQACgkQJdeBCYSNAAP2+QCgkRtmKt2F+DDP1+SmLAQNDUTD
9lQAn0Rxvuw4TIsYiF9KHtmlJDQ2tWuQ
=po4T
-----END PGP SIGNATURE-----

review: Needs Fixing

« Back to merge proposal