• 
      

    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!