flake8
-
rbtools/clients/mercurial.py (Diff revision 1) Show all issues -
Review Request #10101 — Created July 24, 2018 and submitted
Information | |
---|---|
brennie | |
RBTools | |
master | |
7b3cc01... | |
Reviewers | |
rbtools | |
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) |