[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