Bazaar SCM tool support

Review Request #296 — Created March 15, 2008 and submitted

Information

Review Board SVN (deprecated)

Reviewers

Adds Bazaar (bzr) SCM tool support. It is implemented simply by invoking external application (like the cvs support too). However, Bazaar diffs do not contain revision numbers but timestamps. The solution is to find the corresponding revision number in parse_diff_revision function using bzr log.

This is my very first piece of Python code. I submit this, since there might be some other Bazaar users who are willing to participate or help. All comments are welcome. :) 
Tested with simple Bazaar repository. I invite all Bazaar users to help testing this.
chipx86
  1. Thanks for working on this. It's a good start but there's some things to fix up.
    1. Thanks for the comments. Attached is a new version of Bazaar tool support.
  2. reviewboard/scmtools/bzr.py (Diff revision 1)
     
     
     
     
     
    Module names should be alphabetized, and the imports for datetime and timedelta should go on the same line (from datetime impor datetime, timedelta).
  3. reviewboard/scmtools/bzr.py (Diff revision 1)
     
     
     
     
    Two blank lines before the class definition.
    
    Could you briefly document the BZRTool, explain its purpose?
  4. reviewboard/scmtools/bzr.py (Diff revision 1)
     
     
    The print statements should go away.
  5. reviewboard/scmtools/bzr.py (Diff revision 1)
     
     
    Excess trailing whitespace. Here and elsewhere in the file.
  6. reviewboard/scmtools/bzr.py (Diff revision 1)
     
     
     
    What's the difference between rev and revision? Could you add a comment explaining this?
  7. reviewboard/scmtools/bzr.py (Diff revision 1)
     
     
    These look indented too far.
    
    Also, I'm not wild about referencing the values by position. Can you instead split the line and check the parts for "revno:" and "timestamp:" and grab the value that way?
  8. reviewboard/scmtools/bzr.py (Diff revision 1)
     
     
     
    There's probably a better way to structure this. There's a lot of magic numbers for positions in here. What's this supposed to do?
  9. reviewboard/scmtools/bzr.py (Diff revision 1)
     
     
    Mix of spaces and tabs for the leading whitespace. It should be spaces only.
  10. 
      
HH
  1. This is my second attempt.
  2. reviewboard/scmtools/bzr.py (Diff revision 2)
     
     
    Unfortunately this system did not let me update tbe diff twice, and thus these extra spaces are still here.
    1. You should be able to. It's possible you tried while we were having server problems. Can you try again?
  3. 
      
david
  1. I went ahead and made various style changes and committed this as r1300.  Thanks!
    1. Excellent! It makes life much easier to have this in the standard distribution rather than a patch. Thank you very much.
      
      I have found an issue, tough. Bazaar does not support the cat command if a repository is accessed through a sftp connection. The only way to handle this seems to be checking out the whole repository and then performing the cat command from that temporary repository. Because that is too heavy operation, and the current solution works in most of the cases, I have left the code as it is.
  2. reviewboard/scmtools/bzr.py (Diff revision 2)
     
     
    This should be up in the standard-library group, alphabetically.
  3. 
      
Loading...