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

Information

Review Board SVN (deprecated)

Reviewers

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.
david
  1. Any ETA on the new diff?
  2. 
      
NO
OR
  1. 
      
  2. 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.
  3. /trunk/reviewboard/contrib/tools/reviewboard-post-commit-hook.py (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    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.
    1. Both good suggestions, thanks.  I've made them in the latest diff.
  4. 
      
chipx86
  1. 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.
    1. I agree that the inline commands aren't particularly natural, but this script was started as an extension of the trac-post-commit svn script, which uses a similar command structure.  And, as you guessed, this script is what we've come to rely on at my company.  I'm happy to make it available as a third-party contrib script.
      
      Thanks for the suggestions; I've made all the changes save one (commented below).
  2. 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.
  3. Space on both sides of "="
  4. Space on both sides of "="
    
    Same with other variables in this file.
  5. Should just do: if not re.search(...):
  6. sys.stderr.write?
    1. It's not an error, just information, since some commits don't need reviews.
  7. Should wrap this in a try/except to make sure it is indeed an integer.
  8. Maybe offer a variable near the beginning to set this.
  9. 
      
NO
Review request changed
YU
  1. 
      
  2. How to Use this?thanks!

  3. 
      
Loading...