OutputWrapper class to Commands which outputs to stream object of choice

Review Request #11401 — Created Jan. 23, 2021 and submitted

ryankang
RBTools
master
11521
rbtools, students

RBTools commands were previously using print and 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 for OutputWrapper and another test that makes
sure OutputWrapper passes the correct message to the output stream object

Summary
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
replaced all of the prints with output wrapper and initiated wrapper in Command init
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()
Fixed white space error at the end of rbtools/commands/__init__.py
changed print statements in __init__.py to use output wrapper
fixed Review Bot issues where lines where over 80 characters in __init__.py
moved OutputWrapper tests from __init__.py to test_main.py and renamed test class to OutputWrapperTests
fixed indentation and misspelled words issues in test_main.py:OutputWrapper
moved OutputWrapper tests to test_main.py and imported KGB
Added docstrings for OutputWrapper methods
Changed docstrings to follow Review Board documentation guide.
Fixed multiple blank lines with indentation problems
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!

amohapatraamohapatra

Your summary should be much shorter, about 70 columns. Further detail (for example, what tests were added) should be in ...

daviddavid

E501 line too long (85 > 79 characters)

reviewbotreviewbot

E501 line too long (91 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (85 > 79 characters)

reviewbotreviewbot

E501 line too long (88 > 79 characters)

reviewbotreviewbot

E501 line too long (89 > 79 characters)

reviewbotreviewbot

E211 whitespace before '('

reviewbotreviewbot

E501 line too long (91 > 79 characters)

reviewbotreviewbot

E501 line too long (90 > 79 characters)

reviewbotreviewbot

E501 line too long (89 > 79 characters)

reviewbotreviewbot

E501 line too long (88 > 79 characters)

reviewbotreviewbot

E501 line too long (85 > 79 characters)

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

E501 line too long (86 > 79 characters)

reviewbotreviewbot

E501 line too long (83 > 79 characters)

reviewbotreviewbot

E501 line too long (84 > 79 characters)

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E501 line too long (91 > 79 characters)

reviewbotreviewbot

E501 line too long (81 > 79 characters)

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E501 line too long (83 > 79 characters)

reviewbotreviewbot

E501 line too long (88 > 79 characters)

reviewbotreviewbot

E501 line too long (89 > 79 characters)

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

W292 no newline at end of file

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

E124 closing bracket does not match visual indentation

reviewbotreviewbot

E231 missing whitespace after ','

reviewbotreviewbot

E501 line too long (86 > 79 characters)

reviewbotreviewbot

W291 trailing whitespace

reviewbotreviewbot

For consistency with the other changes, should this maybe instead be self.stdout.new_line() ?

amohapatraamohapatra

Again for consistency, maybe this should be self.stdout.new_line() ?

amohapatraamohapatra

This should be self.stdout.new_line()?

amohapatraamohapatra

This should be self.stdout.new_line() ?

amohapatraamohapatra

This should also be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

This should be self.stdout.new_line()

amohapatraamohapatra

W292 no newline at end of file

reviewbotreviewbot

W391 blank line at end of file

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (81 > 79 characters)

reviewbotreviewbot

E501 line too long (89 > 79 characters)

reviewbotreviewbot

E501 line too long (83 > 79 characters)

reviewbotreviewbot

E501 line too long (89 > 79 characters)

reviewbotreviewbot

All test cases should be in files called test_X, not the __init__.py. In this case I think you could probably ...

daviddavid

A few changes in here: The file needs a module docstring The first thing after that docstring should be from ...

daviddavid

Dedent this three spaces.

daviddavid

Typo: strea -> stream

daviddavid

This needs a method docstring.

daviddavid

This needs a method docstring.

daviddavid

This needs a method docstring.

daviddavid

Can we call these _bytes instead of _byte?

daviddavid

Let's add another newline in here, before the format operation. Otherwise it's sometimes easy to miss when scanning quickly through ...

daviddavid

The second line here needs to be indented a bit more: % (review_request.submitter.fullname or review_request.submitter.username))

daviddavid

When we wrap a string, we put the space at the end of the first line. Let's also put the ...

daviddavid

Please undo the changes here (so the space is at the end of the first line).

daviddavid

One thing Christian just pointed out to me is that he's been moving to just import kgb, and then using ...

daviddavid

Please make sure this is sorted alphabetically.

daviddavid

We still need two blank lines here.

daviddavid

This isn't a good description. Let's say "Unit tests for command OutputWrapper"

daviddavid

Docstrings need to be in the following form: """Single-line summary. Multi-line description. """ Along with this, for any newly-introduced classes ...

chipx86chipx86

Function docstrings must follow the same form, and specify any arguments, return values, and exceptions raised. Take a look at ...

chipx86chipx86

We expect a blank line between the end of a block and a new statememt, to help separate that out ...

chipx86chipx86

There's some typos in these docstrings ("charater" here, "specifiy" in the class docstring). If your editor supports it, I recommend ...

chipx86chipx86

This docstring is old, and needs to be modernized. As a step toward this, since you're introducing some really important ...

chipx86chipx86

There's an inconsistency in variable names. You have stderr_bytes (plural) and stderr_byte (singular). The plural form is what we want, ...

chipx86chipx86

Here's an area where the new output wrappers solve a problem! Not all diffs can be decoded as UTF-8, so ...

chipx86chipx86

The string is a parameter to textwrap.fill(), rather than a second parameter to stdout.write. Since we're wrapping to the following ...

chipx86chipx86

To keep lint checkers from complaining about indentations being multiples of 4, this should actually be in the same form ...

chipx86chipx86

This should be a single import statement.

chipx86chipx86

This is a class docstring, so it needs to have a trailing period.

chipx86chipx86

All unit test docstrings should be in the form of Testing <Thing> <condition>. For instance, Testing OutputWrapper initializes stream Testing ...

chipx86chipx86

W293 blank line contains whitespace

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

W293 blank line contains whitespace

reviewbotreviewbot

E126 continuation line over-indented for hanging indent

reviewbotreviewbot

F401 'rbtools.commands.JSONWrapper' imported but unused

reviewbotreviewbot

F821 undefined name 'JSONOutput'

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

ryankang
ryankang
ryankang
  1. 
      
  2. 
      
ryankang
Review request changed

Change Summary:

Fixing Review Bot issues and multiline string formatting

Commits:

Summary
-
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
-
replaced all of the prints with output wrapper and initiated wrapper in Command init
+
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
+
replaced all of the prints with output wrapper and initiated wrapper in Command init
+
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts

Diff:

Revision 2 (+496 -344)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
ryankang
Review request changed

Change Summary:

Added new test for Outputwrapper's new_line() that ensures output stream receives new line character

Commits:

Summary
-
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
-
replaced all of the prints with output wrapper and initiated wrapper in Command init
-
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
-
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
+
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
+
replaced all of the prints with output wrapper and initiated wrapper in Command init
+
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
+
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
+
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called

Diff:

Revision 4 (+527 -355)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

amohapatra
  1. 
      
  2. rbtools/commands/patch.py (Diff revision 4)
     
     

    For consistency with the other changes, should this maybe instead be self.stdout.new_line() ?

  3. rbtools/commands/post.py (Diff revision 4)
     
     

    Again for consistency, maybe this should be self.stdout.new_line() ?

  4. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()?

  5. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line() ?

  6. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should also be self.stdout.new_line()

  7. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  8. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  9. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  10. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  11. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  12. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  13. rbtools/commands/setup_repo.py (Diff revision 4)
     
     

    This should be self.stdout.new_line()

  14. 
      
amohapatra
  1. 
      
  2. All the changes suggested have to do with consistency with self.stdout.new_line(). Other than that, everything else looks good to me!

  3. 
      
ryankang
Review request changed

Change Summary:

Replaced self.stdout.write("\n") with self.stdout.new_line()

Commits:

Summary
-
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
-
replaced all of the prints with output wrapper and initiated wrapper in Command init
-
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
-
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
-
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
+
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
+
replaced all of the prints with output wrapper and initiated wrapper in Command init
+
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
+
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
+
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
+
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()

Diff:

Revision 5 (+543 -369)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
ryankang
ryankang
Review request changed

Commits:

Summary
-
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
-
replaced all of the prints with output wrapper and initiated wrapper in Command init
-
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
-
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
-
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
-
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()
-
Fixed white space error at the end of rbtools/commands/__init__.py
+
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
+
replaced all of the prints with output wrapper and initiated wrapper in Command init
+
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
+
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
+
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
+
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()
+
Fixed white space error at the end of rbtools/commands/__init__.py
+
changed print statements in __init__.py to use output wrapper

Diff:

Revision 7 (+570 -398)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
david
  1. 
      
  2. 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 existing test_main.py file.

    This class should also probably be called OutputWrapperTests.

  3. rbtools/commands/tests/__init__.py (Diff revision 8)
     
     
     
     
     
     

    A few changes in here:

    1. The file needs a module docstring
    2. The first thing after that docstring should be from __future__ import unicode_literals
    3. 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
    
  4. rbtools/commands/tests/__init__.py (Diff revision 8)
     
     

    Dedent this three spaces.

  5. rbtools/commands/tests/__init__.py (Diff revision 8)
     
     

    Typo: strea -> stream

  6. 
      
david
  1. 
      
  2. Your summary should be much shorter, about 70 columns. Further detail (for example, what tests were added) should be in the description.

  3. 
      
ryankang
ryankang
david
  1. 
      
  2. rbtools/commands/__init__.py (Diff revision 10)
     
     

    This needs a method docstring.

  3. rbtools/commands/__init__.py (Diff revision 10)
     
     

    This needs a method docstring.

  4. rbtools/commands/__init__.py (Diff revision 10)
     
     

    This needs a method docstring.

  5. rbtools/commands/__init__.py (Diff revision 10)
     
     
     

    Can we call these _bytes instead of _byte?

  6. rbtools/commands/__init__.py (Diff revision 10)
     
     
     

    Let's add another newline in here, before the format operation. Otherwise it's sometimes easy to miss when scanning quickly through the code.

    self.stdout.write('Please log in to the Review Board server at '
                      '%s.'
                      % urlparse(uri)[1])
    
  7. rbtools/commands/info.py (Diff revision 10)
     
     
     

    The second line here needs to be indented a bit more:

    % (review_request.submitter.fullname or
       review_request.submitter.username))
    
  8. rbtools/commands/land.py (Diff revision 10)
     
     
     

    When we wrap a string, we put the space at the end of the first line. Let's also put the format parameters on their own line:

    self.stdout.write('Recursively landing dependencies of '
                      'review request %s.'
                      % review_request_id)
    
  9. rbtools/commands/patch.py (Diff revision 10)
     
     
     

    Please undo the changes here (so the space is at the end of the first line).

  10. rbtools/commands/status_update.py (Diff revision 10)
     
     
     
     
     
     
     

    The new formatting here is so much nicer! Good work.

  11. rbtools/commands/tests/test_main.py (Diff revision 10)
     
     

    One thing Christian just pointed out to me is that he's been moving to just import kgb, and then using kgb.SpyAgency where necessary. This makes it easier when we want to add things later like spy operations.

  12. rbtools/commands/tests/test_main.py (Diff revision 10)
     
     

    Please make sure this is sorted alphabetically.

  13. rbtools/commands/tests/test_main.py (Diff revision 10)
     
     
     

    We still need two blank lines here.

  14. rbtools/commands/tests/test_main.py (Diff revision 10)
     
     

    This isn't a good description. Let's say "Unit tests for command OutputWrapper"

  15. 
      
ryankang
chipx86
  1. 
      
  2. rbtools/commands/__init__.py (Diff revision 11)
     
     
     
     
     
     

    Docstrings need to be in the following form:

    """Single-line summary.
    
    Multi-line description.
    """
    

    Along with this, for any newly-introduced classes (or new functions/attributes on existing classes), we want to include version information directly below the description, like so:

    """Single-line summary.
    
    Multi-line description.
    
    Version Added:
        3.0
    """
    
  3. rbtools/commands/__init__.py (Diff revision 11)
     
     
     
     
     

    Function docstrings must follow the same form, and specify any arguments, return values, and exceptions raised. Take a look at the docs for this on Notion.

    We do have a lot of old docstrings in RBTools that don't conform to the modern standard, but if you grep around for "Args:" or "Returns:", you'll see examples.

    This comment applies throughout the change.

  4. rbtools/commands/__init__.py (Diff revision 11)
     
     
     

    We expect a blank line between the end of a block and a new statememt, to help separate that out as a new chunk of code.

  5. rbtools/commands/__init__.py (Diff revision 11)
     
     

    There's some typos in these docstrings ("charater" here, "specifiy" in the class docstring). If your editor supports it, I recommend running a spell check.

  6. rbtools/commands/__init__.py (Diff revision 11)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    This docstring is old, and needs to be modernized. As a step toward this, since you're introducing some really important new attributes, let's begin documenting them.

    To do this, add an Attributes section at the end of this docstring:

    """...
    
    ...
    
    Attributes:
        stderr (OutputWrapper):
            Standard error output wrapper that subclasses must write to.
    
        stdout (OutputWrapper):
            ...
    
        ...
    

    Attributes should be in alphabetical order.

  7. rbtools/commands/__init__.py (Diff revision 11)
     
     
     
     
     
     
     
     
     
     

    There's an inconsistency in variable names. You have stderr_bytes (plural) and stderr_byte (singular).

    The plural form is what we want, I think.

    Also, this if/else always sets both, so we have no reason to default to None above.

  8. rbtools/commands/patch.py (Diff revision 11)
     
     
     
     
     
     

    Here's an area where the new output wrappers solve a problem!

    Not all diffs can be decoded as UTF-8, so we want to write as a byte string. This code here was using sys.stdout.buffer.write to let us do that on Python 3, and just print() on Python 2.

    The correct thing will be to replace this entire chunk of logic with a write to self.stdout_bytes.

    Make sure, along with replacing print statements, that you're looking for and replacing sys.stdout or sys.stderr.

  9. rbtools/commands/setup_repo.py (Diff revision 11)
     
     
     
     
     
     
     
     
     
     

    The string is a parameter to textwrap.fill(), rather than a second parameter to stdout.write. Since we're wrapping to the following line to give us room for the text, we need to keep it 4 spaces indented from the start of the main statement. In order words, what we had before was correct.

  10. rbtools/commands/setup_repo.py (Diff revision 11)
     
     
     
     
     
     
     
     

    To keep lint checkers from complaining about indentations being multiples of 4, this should actually be in the same form we had before. The parameters indent to 4 spaces relative to the statement, allowing dictionary keys to indent 4 spaces from that. So you can revert these indentation changes, and keep the string on the second line.

  11. rbtools/commands/tests/test_main.py (Diff revision 11)
     
     
     

    This should be a single import statement.

  12. rbtools/commands/tests/test_main.py (Diff revision 11)
     
     

    This is a class docstring, so it needs to have a trailing period.

  13. rbtools/commands/tests/test_main.py (Diff revision 11)
     
     
     
     

    All unit test docstrings should be in the form of Testing <Thing> <condition>.

    For instance, Testing OutputWrapper initializes stream

    Testing OutputWrapper.write with ...

    Testing OutputWrapper.new_line

    etc.

  14. 
      
ryankang
Review request changed

Commits:

Summary
-
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
-
replaced all of the prints with output wrapper and initiated wrapper in Command init
-
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
-
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
-
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
-
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()
-
Fixed white space error at the end of rbtools/commands/__init__.py
-
changed print statements in __init__.py to use output wrapper
-
fixed Review Bot issues where lines where over 80 characters in __init__.py
-
moved OutputWrapper tests from __init__.py to test_main.py and renamed test class to OutputWrapperTests
-
fixed indentation and misspelled words issues in test_main.py:OutputWrapper
-
moved OutputWrapper tests to test_main.py and imported KGB
-
Added docstrings for OutputWrapper methods
+
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
+
replaced all of the prints with output wrapper and initiated wrapper in Command init
+
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
+
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
+
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
+
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()
+
Fixed white space error at the end of rbtools/commands/__init__.py
+
changed print statements in __init__.py to use output wrapper
+
fixed Review Bot issues where lines where over 80 characters in __init__.py
+
moved OutputWrapper tests from __init__.py to test_main.py and renamed test class to OutputWrapperTests
+
fixed indentation and misspelled words issues in test_main.py:OutputWrapper
+
moved OutputWrapper tests to test_main.py and imported KGB
+
Added docstrings for OutputWrapper methods
+
Changed docstrings to follow Review Board documentation guide.

Diff:

Revision 12 (+752 -500)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
ryankang
Review request changed

Commits:

Summary
-
added OutputWrapper class to Commands which outputs to stream object of choice. Added test suite that checks if OutputWrapper has correct stream and that OutputWrapper passes correct message to stream object
-
replaced all of the prints with output wrapper and initiated wrapper in Command init
-
Fixed all of the Review Bot string formatting issues and fixed consistency issues with strings on multiple lines by aligning their starts
-
Fixed Review Bot issues on trailing white spaces, indentation, whitespace after comma, and lines over 79 characters
-
Added new test for OutputWrapper than ensures it passes a new line character to output stream when new_line() is called
-
Fixed inconsistency of the use of new_line(). Replaced self.stdout.write(n) with self.stdout.new_line()
-
Fixed white space error at the end of rbtools/commands/__init__.py
-
changed print statements in __init__.py to use output wrapper
-
fixed Review Bot issues where lines where over 80 characters in __init__.py
-
moved OutputWrapper tests from __init__.py to test_main.py and renamed test class to OutputWrapperTests
-
fixed indentation and misspelled words issues in test_main.py:OutputWrapper
-
moved OutputWrapper tests to test_main.py and imported KGB
-
Added docstrings for OutputWrapper methods
-
Changed docstrings to follow Review Board documentation guide.
-
Fixed multiple blank lines with indentation problems
+
merged in Output Wrapper Object from r#11401
+
added new method to print JSON to stream object and tests to ensure it works properly
+
added new global command --json
+
added conditional at the end of command cycle to print JSON is enabled
+
added method to append to classes and tests to ensure items are appended correctly
+
added new method to add errors to existing errors key or create a new one
+
Fixed issues with extra white spaces in files from Review Bot
+
Added docstrings to JSONOutput class and changed name from JSONWrapper to JSONOutput
+
Fixed minor formatting issues in test_main.py
+
Updated JSONOutput docstrings to format with guidelines
+
Updated docstrings in JSONOutputTests in test_main.py to comply with guidelines
+
Checking if there are any errors in json to decide status
+
Fixed trailing white space in test_main.test_hson_wrapper_append
+
Fixed blank lines with spaces in __init__ Command.run_from_argv
+
Added any CommandError caught to errors key

Diff:

Revision 14 (+465 -99)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

ryankang
ryankang
david
  1. Making some tweaks and getting this landed. Thanks!

  2. 
      
ryankang
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (db0c1f5)
Loading...