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

Information

Review Board
release-1.7.x

Reviewers

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)
     
     
    Show all issues

    Please wrap this to 79 columns.

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

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

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

    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)
     
     
    Show all issues

    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)
     
     
    Show all issues

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

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

    The parentheses around the variable names aren't necessary.

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

    Please move the import to the top of the file.

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

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

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

    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)
     
     
    Show all issues

    The parentheses around the variable names aren't necessary.

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

    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:
Completed
Change Summary:
Pushed to release-1.7.x (2939338). Thanks!