• 
      

    [WIP] HostingServices: Bitbucket get_remote_repositories, get_commits, get_branches

    Review Request #5450 — Created Feb. 10, 2014 and discarded

    Information

    Review Board
    master

    Reviewers

    Added get_remote_repositories, get_commits, get_branches for Bitbucket

    Command line testing done, unit tests will be added after tests in request #5432 are approved.

    Description From Last Updated

    Same as my other review concern - can we guarantee that this will exit without putting our full trust in …

    mike_conleymike_conley

    No space before the """.

    chipx86chipx86

    Can be one statement.

    chipx86chipx86

    Blank line before this.

    chipx86chipx86

    Blank line before this.

    chipx86chipx86

    BLANK LINE BEFORE THIS.

    chipx86chipx86

    One variable setting per line. Same below.

    chipx86chipx86

    Blank line between these.

    chipx86chipx86

    next is a reserved word.

    chipx86chipx86

    We don't use the if .. else .. form in our code. Should just be a standard if statement.

    chipx86chipx86

    The """ should go on the previous line.

    chipx86chipx86

    Blank line before this.

    chipx86chipx86

    Blank line before this.

    chipx86chipx86

    No u prefixes.

    chipx86chipx86

    And this.

    chipx86chipx86

    A quicker and nicer way of doing this is: results = [ Branch(...) for name, info in six.iteritems(branches) ] You …

    chipx86chipx86

    You can just do: if not results:

    chipx86chipx86

    And this.

    chipx86chipx86

    This line is too long. Maybe shorten the summary and go into more detail in the description.

    chipx86chipx86

    Blank line above.

    chipx86chipx86

    Blank line before this.

    chipx86chipx86

    Did you mean to capitalize all of "THE" ?

    chipx86chipx86

    And this.

    chipx86chipx86

    Blank line before this.

    chipx86chipx86
    mike_conley
    1. 
        
    2. reviewboard/hostingsvcs/bitbucket.py (Diff revision 1)
       
       
      Show all issues

      Same as my other review concern - can we guarantee that this will exit without putting our full trust in Bitbucket to do the right thing?

    3. 
        
    OL
    OL
    OL
    OL
    chipx86
    1. 
        
    2. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      No space before the """.

    3. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
       
      Show all issues

      Can be one statement.

    4. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      Blank line before this.

    5. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      Blank line before this.

    6. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      BLANK LINE BEFORE THIS.

    7. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      One variable setting per line.

      Same below.

    8. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
       
      Show all issues

      Blank line between these.

    9. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      next is a reserved word.

    10. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      We don't use the if .. else .. form in our code. Should just be a standard if statement.

    11. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
       
      Show all issues

      The """ should go on the previous line.

    12. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      Blank line before this.

    13. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      Blank line before this.

    14. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      No u prefixes.

    15. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      And this.

    16. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
       
       
       
      Show all issues

      A quicker and nicer way of doing this is:

      results = [
          Branch(...)
          for name, info in six.iteritems(branches)
      ]
      

      You then won't need to set results = [] above.

      Also note the six.iteritems. We use a module called six to provide compatibility with Python 3. One of the things we must use it for is any iter* function.

      If you do any iteration anywhere else in your code (I may have missed these), please update them to use six.

    17. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      You can just do:

      if not results:
      
    18. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      And this.

    19. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      This line is too long. Maybe shorten the summary and go into more detail in the description.

    20. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      Blank line above.

    21. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      Blank line before this.

    22. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      Did you mean to capitalize all of "THE" ?

    23. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      And this.

    24. reviewboard/hostingsvcs/bitbucket.py (Diff revision 2)
       
       
      Show all issues

      Blank line before this.

    25. 
        
    OL
    OL
    Review request changed
    Status:
    Discarded