flake8
-
rbtools/clients/mercurial.py (Diff revision 1) Show all issues -
Review Request #10101 — Created July 24, 2018 and submitted
The Mercurial SCM tool can now generate commit histories so that it can
be used to create review requests with history.
Posted review requests with history.
Description | From | Last Updated |
---|---|---|
E303 too many blank lines (2) |
![]() |
|
F821 undefined name 'NO_PARENT' |
![]() |
|
Byte string prefix? We'd need it above as well. |
|
|
Byte string prefix? |
|
|
Alphabetical order. |
|
|
This should say that it raises NotImplementedError under the conditions. |
|
|
It'd be nice to know where the revisions come from. |
|
|
Just to check, is \\x1f right, or would \x1f be right? |
|
|
We use these special markers in several places. Let's make them constants so it's readable and consistent. |
|
|
This was reordered but wasn't put in alphabetical order. |
|
|
I think the function name changed in a parent change. Can you retest with that? |
|
|
Must be the full path: rbtools.clients.errors.SCMError. |
|
|
It'd be nice to have these as constants like with Git, to help with readability. |
|
|
Is there any reason why we couldn't just do if entry['parent2'] != self.NO_PARENT? We don't use it elsewhere, and it … |
|
|
Missing period. |
|
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+62) |
Address issues
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+64) |
rebase
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+64) |
rbtools/clients/mercurial.py (Diff revision 4) |
---|
This should say that it raises
NotImplementedError
under the conditions.
rbtools/clients/mercurial.py (Diff revision 4) |
---|
Just to check, is
\\x1f
right, or would\x1f
be right?
rbtools/clients/mercurial.py (Diff revision 4) |
---|
We use these special markers in several places. Let's make them constants so it's readable and consistent.
addressed feedback
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+70 -1) |
rbtools/clients/mercurial.py (Diff revision 5) |
---|
This was reordered but wasn't put in alphabetical order.
rbtools/clients/mercurial.py (Diff revision 5) |
---|
I think the function name changed in a parent change. Can you retest with that?
rbtools/clients/mercurial.py (Diff revision 5) |
---|
Must be the full path:
rbtools.clients.errors.SCMError
.
rbtools/clients/mercurial.py (Diff revision 5) |
---|
It'd be nice to have these as constants like with Git, to help with readability.
addressed feedback
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 6 (+91 -2) |
rbtools/clients/mercurial.py (Diff revision 6) |
---|
Is there any reason why we couldn't just do
if entry['parent2'] != self.NO_PARENT
? We don't use it elsewhere, and it seems like it's always going to be present (pop
will throw an exception by default if not).
addressed feedback
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 7 (+92 -2) |
with_parent_diff
to MercurialClient.diff
parent
vs parent_id
Testing Done: |
|
||||||
---|---|---|---|---|---|---|---|
Commit: |
|
||||||
Diff: |
Revision 8 (+102 -4) |
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 9 (+102 -4) |