flake8
-
rbtools/clients/git.py (Diff revision 1) Show all issues
Review Request #9221 — Created Sept. 24, 2017 and submitted
Information | |
---|---|
bleblan2 | |
RBTools | |
release-0.7.x | |
4551 | |
276ea82... | |
Reviewers | |
rbtools, students | |
chipx86, david |
When creating a git diff with rbt diff, the -M option is automatically
used if available and the user has no way of making rbt pass in
--no-renames
to git diff.This change adds a
--no-renames
flag to the rbt diff command which then
gets passed into git diff.
Tests were run.
Description | From | Last Updated |
---|---|---|
E111 indentation is not a multiple of four |
![]() |
|
Blank line between these. |
|
|
Again, I don't like using extra_args this way. We should definitely be using keyword args for this. |
|
|
no_renames=False over extra_args=[] |
|
|
Can you split this up for one condition per line? It makes it easier to read. |
|
|
Update this to the next point release for RBTools: 0.7.11 |
|
|
Blank line between these. |
|
|
Instead of checking the tool type, we should add a flag to SCMTool like supports_no_renames (defaulting to False) and set … |
|
|
I'm not sure that we should be using extra_args for this. This seems like a job for a keyword argument. |
|
|
These can be combined into a single conditional. |
|
|
I suspect we want to make this a fatal error (i.e. raise CommandError instead of just log something). |
|
Fix flake8 error with indent.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+25 -4) |
rbtools/clients/git.py (Diff revision 2) |
---|
Again, I don't like using
extra_args
this way. We should definitely be using keyword args for this.
rbtools/commands/__init__.py (Diff revision 2) |
---|
Update this to the next point release for RBTools: 0.7.11
rbtools/commands/diff.py (Diff revision 2) |
---|
Instead of checking the tool type, we should add a flag to
SCMTool
likesupports_no_renames
(defaulting to False) and setsupports_no_renames = True
inclients/git.py
. Then we can check:if self.options.no_renames: if tool.supports_no_renames: # ... else: logging.error('The %s SCM tool does not support diffs without renames', tool.type)
rbtools/commands/diff.py (Diff revision 2) |
---|
I'm not sure that we should be using
extra_args
for this. This seems like a job for a keyword argument.
Update to use keyword arg for
no_renames
and address review comments.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+33 -14) |
Teeny tiny nitpicks :) You should probably get either David and/or Christian's feedback about this since we're adding new arguments to diff() and that could break third party SCM clients.
rbtools/clients/git.py (Diff revisions 2 - 3) |
---|
Can you split this up for one condition per line? It makes it easier to read.
Update code style.
Commit: |
|
||||
---|---|---|---|---|---|
People: |
|
||||
Diff: |
Revision 4 (+35 -14) |
I'm happy with this as-is. Will wait for feedback from Chirstian and/or David before landing, however.
rbtools/commands/diff.py (Diff revision 4) |
---|
I suspect we want to make this a fatal error (i.e. raise
CommandError
instead of just log something).
Clean up structure around tool not supporting --no-renames flag based on feedback.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+32 -14) |