Support post-review in Clearcase snapshot view
Review Request #3822 — Created Feb. 6, 2013 and submitted
Information | |
---|---|
moonese | |
RBTools | |
Reviewers | |
rbtools | |
Currrently run RBTools in snapshot view will get error message saying "To generate diff using parent branch or by passing revision ranges, you must use a dynamic view. So I made some change to support snapshot view post-review, as it will be useful for clearcase users working on snapshot views. As new to Python, I think the ugly code I changed could be improved. Anyway, it works. The idea is to use 'cleartool get -to dest-pname pname' command to copy the revision to temp dir and then run diff on temp old new files.
Tested on Clearcase 7.1.2 snapshot view, the diff could be uploaded just like dynamic view.
Description | From | Last Updated |
---|---|---|
too verbose, need improve here ... |
MO moonese | |
Col: 80 E501 line too long (82 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (82 > 79 characters) |
![]() |
|
if diff is dir, which is the proper way to handle them? |
MO moonese | |
Col: 80 E501 line too long (99 > 79 characters) |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 80 E501 line too long (84 > 79 characters) |
![]() |
|
Col: 80 E501 line too long (93 > 79 characters) |
![]() |
|
Col: 57 E702 multiple statements on one line (semicolon) |
![]() |
|
Col: 57 E702 multiple statements on one line (semicolon) |
![]() |
|
Col: 1 W293 blank line contains whitespace |
![]() |
|
Col: 80 E501 line too long (93 > 79 characters) |
![]() |
|
This is kind of crummy (and suffers from the same security problems as mktemp). I'd suggest using tempfile.mkdtemp() to create … |
|
|
I think it would be better to wrap these in try/except: try: os.remove(temp_old_file) except OSError: pass try: os.remove(temp_new_file) except OSError: … |
|
|
Col: 18 E111 indentation is not a multiple of four |
![]() |
|
it should be "tmp_old_file", not "temp_old_file" |
PE penguinol | |
it should be "tmp_new_file", not "temp_new_file" |
PE penguinol | |
File "clearcase.py", line 282, in diff_files AttributeError: 'str' object has no attribute 'name' use "tmp_old_file" instead of "tmp_old_file.name" the same … |
PE penguinol |
Description: |
|
---|
-
-
-
rbtools/clients/clearcase.py (Diff revision 1) if diff is dir, which is the proper way to handle them?
Change Summary:
1. change the way to generate temp file using tempfile.NamedTemporaryFile() 2. some other minor fix
Diff: |
Revision 2 (+20 -8) |
---|

-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbtools/clients/clearcase.py Ignored Files:
-
-
-
-
rbtools/clients/clearcase.py (Diff revision 2) Col: 57 E702 multiple statements on one line (semicolon)
-
rbtools/clients/clearcase.py (Diff revision 2) Col: 57 E702 multiple statements on one line (semicolon)
-

-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbtools/clients/clearcase.py Ignored Files:
-
Change Summary:
fix previous Review Bot Error: E501 line too long (93 > 79 characters)
Diff: |
Revision 4 (+22 -8) |
---|

-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbtools/clients/clearcase.py Ignored Files:
-
-
rbtools/clients/clearcase.py (Diff revision 4) This is kind of crummy (and suffers from the same security problems as mktemp). I'd suggest using tempfile.mkdtemp() to create a temporary directory, and then put your temporary files in there. Make sure to remove that directory at the end.
Change Summary:
use rbtools.utils.filesystem.make_tempfile() to generate temporary files
Diff: |
Revision 5 (+28 -8) |
---|

-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbtools/clients/clearcase.py Ignored Files:
-
Just one last comment from me:
-
rbtools/clients/clearcase.py (Diff revision 5) I think it would be better to wrap these in try/except: try: os.remove(temp_old_file) except OSError: pass try: os.remove(temp_new_file) except OSError: pass That way it's a little more robust against races.

-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbtools/clients/clearcase.py Ignored Files:
-

-
This is a review from Review Bot. Tool: PEP8 Style Checker Processed Files: rbtools/clients/clearcase.py Ignored Files:
-
-
-
-
rbtools/clients/clearcase.py (Diff revision 7) File "clearcase.py", line 282, in diff_files AttributeError: 'str' object has no attribute 'name' use "tmp_old_file" instead of "tmp_old_file.name" the same problem @ line 282,283,284,293,294