Add ability to post a path with Perforce.

Review Request #753 — Created Feb. 26, 2009 and submitted

Information

Review Board SVN (deprecated)

Reviewers

Add ability to post a path with Perforce.

We have some use cases where it would be nice to review entire files post-commit in reviewboard.  Some examples:

- Our documentation needs to be checked in before it is reviewed.  If multiple people are working on it, or if it is created as a series of multiple checkins, reviewing specific change numbers is difficult.  With this, we can do "post-review //docs/main/some/project/...".

- If you work on a side branch for a while, possibly with multiple checkins or with multiple people, reviewing the changes on the side branch before integration to the main branch can be tricky.  With this, you can now do "post-review //path/to/my/branch/...@100,@120" where "100" would be the initial revision that created the branch and "120" is the last revision on that branch.  This allows you to easily review all changes on a branch in one review request.

- If you want to review an old, existing file that has had little or no reviews, you can now do this with "post-review //path/to/some/file" to review the whole thing.

The specific path types supported are:

post-review //path/to/file
# Upload file as a "new" file.

post-review //path/to/dir/...
# Upload all files as "new" files.

post-review //path/to/file[@#]rev
# Upload file from that rev as a "new" file.
# (Not a very useful scenario, but it works.)

post-review //path/to/file[@#]rev,[@#]rev
# Upload a diff between revs.

post-review //path/to/dir/...[@#]rev,[@#]rev
# Upload a diff of all files between revs in that directory.

You can specify multiple paths on the command line.  I didn't use the range revision option because the Perforce path syntax is a little more flexible, especially if you post multiple paths.
Tried all the various path types with added/modified/deleted files.
Also made sure didn't break existing changenum posting.
chipx86
  1. Hey Eric. Thanks for the change. Mostly there's stylistic things to address, and I want to test this out a bit on our install just to make sure I don't have a company full of angry engineers next time we update :) Sorry for the delays in this.
  2. Could you add a Python doc string to this explaining the purpose of the function?
  3. Can we make this wrap to < 80 chars? Also, should be r'...' instead of just '...' for regexes.
  4. Blank line before/after blocks.
  5. Blank line after this block.
  6. Same.
  7. /trunk/reviewboard/contrib/tools/post-review (Diff revision 1)
     
     
     
     
    Should also do a blank line before each comment block.
  8. Blank line before this.
  9. Blank line before this, and can you make it wrap to < 80 chars?
  10. Blank line before this.
  11. Blank line before this (after the while block).
  12. /trunk/reviewboard/contrib/tools/post-review (Diff revision 1)
     
     
     
     
     
     
     
    Blank lines around blocks here too (except when immediately inside a new block, like the "for" and inner "if"
  13. Delete the blank line, and can you add a Python doc string?
  14. Mark this with a "TODO:" so we can scan for this later.
  15. Should wrap to < 80 chars.
  16. Blank line before this.
  17. 
      
EH
EH
Review request changed

Change Summary:

Migrating my changes from contrib/tools to rbtools (no other changes).

Diff:

Revision 3 (+254 -81)

Show changes

chipx86
  1. Cool, thanks. Committed as r1903.
  2. 
      
Loading...