clearcase review requests often fail in multi-site mode because oid are not available yet on server (reviewboard)

Review Request #5185 — Created Jan. 3, 2014 and submitted

delyn
Review Board
release-1.7.x
3183
reviewboard

when people are submitting review request, oid of oldfile version and new file version are sent and used to retrieve file name.
However oldfile name is usually synced not but newfile one
Since filename is not mandatory, we can catch this exception and try to retrieve path from client filename and vob

create successfully review request before replica is sent

Description From Last Updated

Please wrap this to 79 columns.

daviddavid

Please make sure that all sentences start with a capital letter.

daviddavid

I'd rather import logging at the top of the file so we don't have to do it every time this ...

daviddavid

Please pass format arguments as additional arguments to logging.* Please also make sure this wraps to 79 columns.

daviddavid

Please move the format arguments to be additional arguments to logging.*

daviddavid

The parentheses around the variable names aren't necessary.

daviddavid

Please move the import to the top of the file.

daviddavid

Please pass format arguments as additional arguments to logging.*. Otherwise it'll do two format operations.

daviddavid

Idiomatic python would be while True:

daviddavid

The parentheses around the variable names aren't necessary.

daviddavid

Please keep these in alphabetical order.

daviddavid
david
  1. 
      
  2. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     

    Please wrap this to 79 columns.

  3. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     

    Please make sure that all sentences start with a capital letter.

  4. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     

    I'd rather import logging at the top of the file so we don't have to do it every time this method is called.

  5. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     

    Please pass format arguments as additional arguments to logging.*

    Please also make sure this wraps to 79 columns.

  6. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     

    Please move the format arguments to be additional arguments to logging.*

  7. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     

    The parentheses around the variable names aren't necessary.

  8. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     

    Please move the import to the top of the file.

  9. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     
     

    Please pass format arguments as additional arguments to logging.*. Otherwise it'll do two format operations.

  10. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     

    Idiomatic python would be while True:

    1. Fyi, I've found someone saying while 1 is/was optimized in python compared to while true (French test)
      but I agree while true is more readable, and performances are not critical here
      http://gaminghacks.free.fr/Python%20while%20True%20performance.php

  11. reviewboard/scmtools/clearcase.py (Diff revision 1)
     
     

    The parentheses around the variable names aren't necessary.

  12. 
      
DE
david
  1. 
      
  2. reviewboard/scmtools/clearcase.py (Diff revision 2)
     
     
     
     
     
     

    Please keep these in alphabetical order.

  3. 
      
david
  1. Oh, I also have a question about compatibility. Because there are both reviewboard and rbtools changes to fix this bug, is this code cross-compatible between versions? For example, if I'm using a Review Board instance with the code and RBTools without, or vice-versa, will things break?

    1. Reviewboard and rbtools changes are independants
      rbtools changes will prevent sending version containing /branch/0 (it is not sufficient to fix the issue reported, but reduce number or errors)
      reviewboard changes improve way oid2filename is managed

  2. 
      
DE
david
  1. Ship It!

  2. 
      
DE
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-1.7.x (2939338). Thanks!
Loading...