[MERGE] submit revision spec
John Arbash Meinel
john at arbash-meinel.com
Sun Mar 11 15:34:27 GMT 2007
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.
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)
2) There should be whitespace between the help text and the 'prefix =
"submit:"' line.
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.
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.
4) I would do 1 space between your if/else setup and the next code
inside _match_on. (You currently have 2 blank lines).
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:'
For details, see:
http://bundlebuggy.aaronbentley.com/request/%3C45F38CE3.9020800%40utoronto.ca%3E
More information about the bazaar
mailing list