[MERGE] submit revision spec

[Aaron Bentley]

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

John Arbash Meinel wrote:
> John Arbash Meinel has voted +1 (conditional).
> Status is now: Conditionally approved
> Comment:
> I'm fine with the 'bzr diff -r submit:' though it seems a little funny
> that nothing comes *after* the ':'.
> 
> There are a few other things, though.

Thanks for your review.  Submitted with changes.

> 1) You re-use the RevisionSpec_ancestor class name, it should be
> RevisionSpec_submit. (It works because it registers itself based on the
> 'prefix', but we really should keep the names distinct)

Oops!  Fixed.

> 2) There should be whitespace between the help text and the 'prefix =
> "submit:"' line.

Done.

> 3) I wonder if we want to fall back on the parent branch. I'm sort of
> okay with it, but I always wonder if people would grow to expect it to
> diff against a specific branch, and they do something seemingly
> unrelated, and what they are used to starts changing.

This is pretty much the same as the bundle command, which is half the
point of it.  If the user hasn't specified a submit branch, I think
using the parent branch is likely to be useful.


> Maybe if we just add a note of "diffing against %s branch: %s' %
> (parent_or_submit, url)
> 
> I don't know a good way to pass that information around. So maybe just:
> 
> note('Using %s branch: %s', parent_or_submit, branch.base) inside the
> RevisionSpec_submit code.

I think it's confusing to include the branch.base, but I've added a
message like:
Using parent branch sftp://balrog/~/bzrplugins/inactive/bzrtools-subtree/ or
Using submit branch /home/abentley/bzrrepo/others/bzr.dev

> 4) I would do 1 space between your if/else setup and the next code
> inside _match_on. (You currently have 2 blank lines).

Done / obsoleted by 5.

> 5) Can we refactor RevisionSpec_ancestor so that we can re-use the code,
> rather than having to copy-and-paste most of it? Just inherit and either
> pass the open branch, or even pass the 'submit_location' instead of the
> value after the 'ancestor:'

Done.

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

iD8DBQFF9GYn0F+nu1YWqI0RAojoAJsH9L4f9ovViKkn73vUfLLe5EAO3wCff711
R9S7rHjMRoOEIwrpvdVNzyQ=
=XU0k
-----END PGP SIGNATURE-----