rbtools support diff filename instead of calling scm to generate diff
Review Request #1197 — Created Nov. 5, 2009 and submitted
Information | |
---|---|
clach04 | |
RBTools | |
Reviewers | |
reviewboard | |
chipx86 |
Added new flag "--diff-filename" that specifies the name of the file where diffs should be read from instead of using SCM client to generate diff. Due to the way some (git) SCMClient subclasses change the current directory the new code has to take a backup of the starting directory if diff filename is not fully specified/qualified. NOTE at this time only reading from a file is supported.
git diff --full-index HEAD >mydiff ./postreview.py --server=http://reviews.reviewboard.org/ --diff-filename mydiff -r 1197 I.e. this review was posted with the new code
-
-
rbtools/postreview.py (Diff revision 1) I'd prefer a simpler comment: "upload an existing diff file, instead of generating a new diff"
-
rbtools/postreview.py (Diff revision 1) Combine these. Blank line after these and before the if statement.
-
rbtools/postreview.py (Diff revision 1) This can be simplified a little, and also needs to close the file handle. Should look more like: if options.diff_filename == '-': raise NotImplemented(...) parent_diff = None try: fp = open(os.path.join(origcwd, options.diff_filename), 'r') diff = fp.read() fp.close() except IOError, e: die("Unable to open diff filename: %s" % e)
CL
Change Summary:
Style change to file IO along with help text update based on Christian's feedback.
Testing Done: |
|
|||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+19 -1) |
LO
-
-
rbtools/postreview.py (Diff revision 2) You could combine these 2 lines into elif options.diff_filename: to be consistent with the previous elif. But then, the else: on line 2663 needs to be properly indented.
CL
Change Summary:
Based on feedback, merged if/else (reduces line count and simplifies). Also added commented out code for "-" support. Again this change was posted using the new feature: git diff --full-index HEAD >mynewdiff ./postreview.py --server=http://reviews.reviewboard.org/ --diff-filename mynewdiff -r 1197
Diff: |
Revision 3 (+17) |
---|
CL
Change Summary:
Essentially the same code as the last posted diff, but this is after a "git pull/merge" to latest code as of 2009-01-12. I.e. fully merged code with headrevs.
Diff: |
Revision 4 (+17) |
---|
LO
-
Looks great. I implemented locally a similar change, so it'll be useful if yours goes into the tree. Many thanks.