flake8
-
rbtools/commands/__init__.py (Diff revision 1) Show all issues -
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
Review Request #11401 — Created Jan. 23, 2021 and updated
Information | |
---|---|
ryankang | |
RBTools | |
master | |
Reviewers | |
rbtools, students | |
RBTools commands were previously using
sys.stderr
to output messages. This change abstracting outputting by using a
wrapper around a stream output object. This makes it easier to customize
output streams and suppress output. 4 wrappers (2 for standard output
unicode and byte, 2 for standard error unicode and byte) are initiated
in the Command class that is accessible to all child classes.
Ran all tests in
./tests/runtests.py rbtools.commands
and passed.Added two new tests for
__init__.py
. One that tests if output stream
object is set correctly forOutputWrapper
and another test that makes
sureOutputWrapper
passes the correct message to the output stream object
Summary |
---|
Description | From | Last Updated |
---|---|---|
All the changes suggested have to do with consistency with self.stdout.new_line(). Other than that, everything else looks good to me! |
|
|
Your summary should be much shorter, about 70 columns. Further detail (for example, what tests were added) should be in ... |
|
|
E501 line too long (85 > 79 characters) |
![]() |
|
E501 line too long (91 > 79 characters) |
![]() |
|
W293 blank line contains whitespace |
![]() |
|
E501 line too long (85 > 79 characters) |
![]() |
|
E501 line too long (88 > 79 characters) |
![]() |
|
E501 line too long (89 > 79 characters) |
![]() |
|
E211 whitespace before '(' |
![]() |
|
E501 line too long (91 > 79 characters) |
![]() |
|
E501 line too long (90 > 79 characters) |
![]() |
|
E501 line too long (89 > 79 characters) |
![]() |
|
E501 line too long (88 > 79 characters) |
![]() |
|
E501 line too long (85 > 79 characters) |
![]() |
|
E501 line too long (84 > 79 characters) |
![]() |
|
E501 line too long (86 > 79 characters) |
![]() |
|
E501 line too long (83 > 79 characters) |
![]() |
|
E501 line too long (84 > 79 characters) |
![]() |
|
E501 line too long (80 > 79 characters) |
![]() |
|
E501 line too long (91 > 79 characters) |
![]() |
|
E501 line too long (81 > 79 characters) |
![]() |
|
E501 line too long (80 > 79 characters) |
![]() |
|
E501 line too long (83 > 79 characters) |
![]() |
|
E501 line too long (88 > 79 characters) |
![]() |
|
E501 line too long (89 > 79 characters) |
![]() |
|
E501 line too long (80 > 79 characters) |
![]() |
|
W292 no newline at end of file |
![]() |
|
W291 trailing whitespace |
![]() |
|
E124 closing bracket does not match visual indentation |
![]() |
|
E231 missing whitespace after ',' |
![]() |
|
E501 line too long (86 > 79 characters) |
![]() |
|
W291 trailing whitespace |
![]() |
|
For consistency with the other changes, should this maybe instead be self.stdout.new_line() ? |
|
|
Again for consistency, maybe this should be self.stdout.new_line() ? |
|
|
This should be self.stdout.new_line()? |
|
|
This should be self.stdout.new_line() ? |
|
|
This should also be self.stdout.new_line() |
|
|
This should be self.stdout.new_line() |
|
|
This should be self.stdout.new_line() |
|
|
This should be self.stdout.new_line() |
|
|
This should be self.stdout.new_line() |
|
|
This should be self.stdout.new_line() |
|
|
This should be self.stdout.new_line() |
|
|
This should be self.stdout.new_line() |
|
|
W292 no newline at end of file |
![]() |
|
W391 blank line at end of file |
![]() |
|
W293 blank line contains whitespace |
![]() |
|
E501 line too long (81 > 79 characters) |
![]() |
|
E501 line too long (89 > 79 characters) |
![]() |
|
E501 line too long (83 > 79 characters) |
![]() |
|
E501 line too long (89 > 79 characters) |
![]() |
|
All test cases should be in files called test_X, not the __init__.py. In this case I think you could probably ... |
|
|
A few changes in here: The file needs a module docstring The first thing after that docstring should be from ... |
|
|
Dedent this three spaces. |
|
|
Typo: strea -> stream |
|
rbtools/commands/__init__.py (Diff revision 1) |
---|
Summary: |
|
|||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
Fixing Review Bot issues and multiline string formatting
Commits: |
|
||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+496 -344) |
Commits: |
|
||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+506 -352) |
Added new test for Outputwrapper's new_line() that ensures output stream receives new line character
Commits: |
|
||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 4 (+527 -355) |
rbtools/commands/patch.py (Diff revision 4) |
---|
For consistency with the other changes, should this maybe instead be self.stdout.new_line() ?
rbtools/commands/post.py (Diff revision 4) |
---|
Again for consistency, maybe this should be self.stdout.new_line() ?
Replaced self.stdout.write("\n") with self.stdout.new_line()
Commits: |
|
||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 5 (+543 -369) |
Commits: |
|
||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+542 -370) |
Changed status from work in progress to in review
Summary: |
|
|||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
Commits: |
|
||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 7 (+570 -398) |
Commits: |
|
||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 8 (+579 -405) |
rbtools/commands/tests/__init__.py (Diff revision 8) |
---|
All test cases should be in files called
test_X
, not the__init__.py
. In this case I think you could probably add your new test class to the existingtest_main.py
file.This class should also probably be called
OutputWrapperTests
.
rbtools/commands/tests/__init__.py (Diff revision 8) |
---|
A few changes in here:
- The file needs a module docstring
- The first thing after that docstring should be
from __future__ import unicode_literals
- Imports should be structured in three groups: standard library, third-party libraries, and then rbtools. In each group we alphabetize the imports. So these should look like:
import sys from kgb import SpyAgency from rbtools.commands import OutputWrapper from rbtools.utils.testbase import RBTestBase
Summary: |
|
||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 9 (+615 -449) |
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 10 (+617 -449) |