-
-
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?
[WIP] HostingServices: Bitbucket get_remote_repositories, get_commits, get_branches
Review Request #5450 — Created Feb. 10, 2014 and discarded
Information | |
---|---|
olessia | |
Review Board | |
master | |
Reviewers | |
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 … |
|
|
No space before the """. |
|
|
Can be one statement. |
|
|
Blank line before this. |
|
|
Blank line before this. |
|
|
BLANK LINE BEFORE THIS. |
|
|
One variable setting per line. Same below. |
|
|
Blank line between these. |
|
|
next is a reserved word. |
|
|
We don't use the if .. else .. form in our code. Should just be a standard if statement. |
|
|
The """ should go on the previous line. |
|
|
Blank line before this. |
|
|
Blank line before this. |
|
|
No u prefixes. |
|
|
And this. |
|
|
A quicker and nicer way of doing this is: results = [ Branch(...) for name, info in six.iteritems(branches) ] You … |
|
|
You can just do: if not results: |
|
|
And this. |
|
|
This line is too long. Maybe shorten the summary and go into more detail in the description. |
|
|
Blank line above. |
|
|
Blank line before this. |
|
|
Did you mean to capitalize all of "THE" ? |
|
|
And this. |
|
|
Blank line before this. |
|
Change Summary:
Added get_commits, get_branches
Description: |
|
||||||
---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+111) |
-
-
-
-
-
-
-
-
-
-
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. -
-
-
-
-
-
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 calledsix
to provide compatibility with Python 3. One of the things we must use it for is anyiter*
function.If you do any iteration anywhere else in your code (I may have missed these), please update them to use
six
. -
-
-
reviewboard/hostingsvcs/bitbucket.py (Diff revision 2) This line is too long. Maybe shorten the summary and go into more detail in the description.
-
-
-
-
-