Summary

postreview.py: add separator between commits

Review Request #1465 — Created March 8, 2010 and discarded

Information

Owner

mw_triad

Repository

RBTools

Branch

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

postreview.py: add separator between commits

When posting a git / git-svn review with --guess-description, add a
separator between commit logs. The separator is a 72-character line of a
user-specified character. The default is empty.

Testing Done

posting this review request :-) (with no separator; also tested specifying separator with a git-svn repo, trying both single-commit and multiple-commit requests)

Would you mind also updating the post-review docs in the Review Board source tree with the new parameter?

mw_triad March 10, 2010, 3:36 a.m.

Okay. That is a different repo, though? Or...?

mw_triad March 10, 2010, 3:59 a.m.

Okay, I found them, will post another request presently.

mw_triad March 10, 2010, 4:52 a.m.

Had some problems with that. FTR, I e-mailed the patch to Christian instead.

mw_triad April 8, 2010, 7:37 a.m.

Okay, managed to get reviewboard checked out from github instead of gitorious; doc changes are now up here: http://reviews.reviewboard.org/r/1509/

rbtools/postreview.py (Diff revision 1)

Blank lines around conditionals.

rbtools/postreview.py (Diff revision 1)

Can do: sep = "".ljust(...)

Or: sep = 72 * options.commit_separator

which should do the same thing.

mw_triad March 10, 2010, 3:36 a.m.

Thanks for the tip... my Python is still a bit green :-).

rbtools/postreview.py (Diff revision 1)

Would be nice to guard against the user having specified multiple characters.

rbtools/postreview.py (Diff revision 1)
```
Same here.
```

Change Summary:

Update based on comments... need to find "post-review docs in the Review Board source tree" yet, but hopefully the .py is okay now. Thanks again for the feedback!

Diff:

Revision 2 (+31 -10)

Show changes

rbtools/postreview.py

Looks good for the most part.

Mind writing a unit test?

MW

mw_triad May 6, 2010, 3:57 a.m.
```
How would I go about that?
```

rbtools/postreview.py (Diff revision 2)

I think you mean % (sep or "")

mw_triad May 6, 2010, 3:04 a.m.

No, I definitely don't :-). I want a literal '%s' as part of the pretty-format string ('git log' will put the subject there), and I want 'sep', if it is set, concatenated to the format string.

rbtools/postreview.py (Diff revision 2)

Can you rename this to --git-commit-separator?

Going to try to start prefixing parameters with their SCM until they're "promoted" to working with more.

mw_triad May 6, 2010, 3:56 a.m.

Won't that be a compatibility headache in the future, though? Or do you keep the option around both with and without 'git-'?