SVN post-commit hook script to translate commit message into command line parameters for post-review script
Review Request #589 — Created Oct. 10, 2008 and submitted
My company performs post-commit reviews, so I have written a Subversion post-commit hook script that translates a log message into parameters for the post-review script. post-review in turn acts upon a repository URL. This script is based off of trac-post-commit-hook.py, and recognizes the similar commands for pulling bug numbers. This script relies on changes to post-review submitted in review 609. It also allows a base path to be supplied, which is not yet handled by post-review but for which I will submit another review request.
Posted review requests to local review board instance using configured SVN repository URL. Tested with subversion 1.4.4 and 1.5.2.
- Change Summary:
-
Added a REPOSITORY_URL configuration (see review request 609). Reduced the list of recognized trac-post-commit-hook.py commands to just those for code review. Using these commands (e.g. "draft review ticket:123") will add bug numbers to the "bugs" field in review board. The post-review script is now called via subprocess.Popen, rather than requiring it to be renamed and loaded as a module. Modified the recognized commit message commands to make them somewhat more natural (e.g. "after revision" rather than "prevrev").
- Description:
-
~ My company performs post-commit reviews, so I have written a Subversion post-commit hook script that translates a log message into parameters for the post-review script. post-review in turn acts upon a repository URL. This script is based off of trac-post-commit-hook.py, and recognizes the same commands for pulling bug numbers.
~ My company performs post-commit reviews, so I have written a Subversion post-commit hook script that translates a log message into parameters for the post-review script. post-review in turn acts upon a repository URL. This script is based off of trac-post-commit-hook.py, and recognizes the similar commands for pulling bug numbers.
~ post-review must be renamed or copied to postreview.py so that this script can load it as a module. I suppose an alternative could be calling post-review as is via subprocess.Popen.
~ This script relies on changes to post-review submitted in review 609. It also allows a base path to be supplied, which is not yet handled by post-review but for which I will submit another review request.
- - This script relies on changes to post-review submitted in reviews 587 and 588.
- - 10/24/08: I have made several changes to this script, and will post a new diff to this request once I've fully tested it (let me know if I should instead discard this request and create a new one).
-
-
Can you not just pull this value from the $1 argument (well, after prefixing it with "file://")? That way this script isn't repository-specific, which is handy for people who have a central directory of SVN hooks and symlink them into multiple repositories.
-
This seems like it'd be nicer to just set the close_fds boolean based on the sys.platform and only have one copy of the Popen code.
-
Lots of small things here, but I also want to think about the design. What seems odd to me is the command-oriented way of updating review requests. It feels sort of unnatural in commits to list such commands, because they become a part of the code's history. I can understand stuff like: "Reviewed at http://reviews.example.com/r/123/" (Which is what we use here) or "Review request: 123" or "Closes review request 123" Those are pretty natural. I want to see if we can come up with a design that's more natural in this way. Of course, if this is something you guys are used to, that's perfectly fine. What we could do is host this as a third party commit script, rather than making it an official one, and people can choose to use it if they need advanced syntax like this. I'm going to want David to weigh in on this as well.
-
Trailing whitespace. Also, I feel like we should use the logic we use for Perforce, where we scan up to the first ".", or up to a max length, whichever comes first.
-
-
-
-
-
-
-
-
-
-
-