[WIP] HostingServices: Bitbucket get_remote_repositories, get_commits, get_branches

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

olessia
Review Board
master
reviewboard, students
chipx86

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)
     
     

    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)
     
     

    No space before the """.

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

    Can be one statement.

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

    Blank line before this.

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

    Blank line before this.

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

    BLANK LINE BEFORE THIS.

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

    One variable setting per line.

    Same below.

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

    Blank line between these.

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

    next is a reserved word.

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

    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)
     
     
     

    The """ should go on the previous line.

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

    Blank line before this.

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

    Blank line before this.

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

    No u prefixes.

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

    And this.

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

    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)
     
     

    You can just do:

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

    And this.

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

    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)
     
     

    Blank line above.

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

    Blank line before this.

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

    Did you mean to capitalize all of "THE" ?

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

    And this.

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

    Blank line before this.

  25. 
      
OL
OL
Review request changed

Status: Discarded

Loading...