- Download Diff

Summary:

OutputWrapper class to Commands which outputs to stream object of choice

Review Request #11401 — Created Jan. 23, 2021 and submitted Aug. 30, 2021, 1:47 p.m.

Information
Owner:	ryankang
Repository:	RBTools
Branch:	master
Bugs:
Depends On:
Blocks:	11521
Reviewers
Groups:	rbtools, students
People:

Description

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.

Testing Done

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

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

Screenshots

Issues

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!	amohapatra	Feb. 13, 2021, 2:51 p.m.
Your summary should be much shorter, about 70 columns. Further detail (for example, what tests were added) should be in ...	david	March 2, 2021, 9 p.m.
E501 line too long (85 > 79 characters)	reviewbot	Jan. 24, 2021, 8:11 a.m.
E501 line too long (91 > 79 characters)	reviewbot	Jan. 24, 2021, 8:11 a.m.
W293 blank line contains whitespace	reviewbot	Jan. 24, 2021, 8:13 a.m.
E501 line too long (85 > 79 characters)	reviewbot	Jan. 24, 2021, 8:14 a.m.
E501 line too long (88 > 79 characters)	reviewbot	Jan. 24, 2021, 8:33 a.m.
E501 line too long (89 > 79 characters)	reviewbot	Jan. 24, 2021, 8:33 a.m.
E211 whitespace before '('	reviewbot	Jan. 24, 2021, 8:35 a.m.
E501 line too long (91 > 79 characters)	reviewbot	Jan. 24, 2021, 8:39 a.m.
E501 line too long (90 > 79 characters)	reviewbot	Jan. 24, 2021, 8:39 a.m.
E501 line too long (89 > 79 characters)	reviewbot	Jan. 24, 2021, 8:57 a.m.
E501 line too long (88 > 79 characters)	reviewbot	Jan. 24, 2021, 8:57 a.m.
E501 line too long (85 > 79 characters)	reviewbot	Jan. 24, 2021, 8:57 a.m.
E501 line too long (84 > 79 characters)	reviewbot	Jan. 24, 2021, 8:57 a.m.
E501 line too long (86 > 79 characters)	reviewbot	Feb. 6, 2021, 11:46 a.m.
E501 line too long (83 > 79 characters)	reviewbot	Feb. 6, 2021, 11:46 a.m.
E501 line too long (84 > 79 characters)	reviewbot	Feb. 6, 2021, 11:46 a.m.
E501 line too long (80 > 79 characters)	reviewbot	Feb. 6, 2021, 11:46 a.m.
E501 line too long (91 > 79 characters)	reviewbot	Feb. 6, 2021, 11:46 a.m.
E501 line too long (81 > 79 characters)	reviewbot	Feb. 6, 2021, 11:47 a.m.
E501 line too long (80 > 79 characters)	reviewbot	Feb. 6, 2021, 11:47 a.m.
E501 line too long (83 > 79 characters)	reviewbot	Feb. 6, 2021, 11:47 a.m.
E501 line too long (88 > 79 characters)	reviewbot	Feb. 6, 2021, 11:47 a.m.
E501 line too long (89 > 79 characters)	reviewbot	Feb. 6, 2021, 11:48 a.m.
E501 line too long (80 > 79 characters)	reviewbot	Feb. 6, 2021, 11:48 a.m.
W292 no newline at end of file	reviewbot	Feb. 6, 2021, 11:48 a.m.
W291 trailing whitespace	reviewbot	Feb. 6, 2021, 11:52 a.m.
E124 closing bracket does not match visual indentation	reviewbot	Feb. 6, 2021, 11:52 a.m.
E231 missing whitespace after ','	reviewbot	Feb. 6, 2021, 11:52 a.m.
E501 line too long (86 > 79 characters)	reviewbot	Feb. 6, 2021, 11:53 a.m.
W291 trailing whitespace	reviewbot	Feb. 6, 2021, 11:56 a.m.
For consistency with the other changes, should this maybe instead be self.stdout.new_line() ?	amohapatra	Feb. 13, 2021, 2:54 p.m.
Again for consistency, maybe this should be self.stdout.new_line() ?	amohapatra	Feb. 13, 2021, 2:54 p.m.
This should be self.stdout.new_line()?	amohapatra	Feb. 13, 2021, 2:53 p.m.
This should be self.stdout.new_line() ?	amohapatra	Feb. 13, 2021, 2:53 p.m.
This should also be self.stdout.new_line()	amohapatra	Feb. 13, 2021, 2:53 p.m.
This should be self.stdout.new_line()	amohapatra	Feb. 13, 2021, 2:53 p.m.
This should be self.stdout.new_line()	amohapatra	Feb. 13, 2021, 2:53 p.m.
This should be self.stdout.new_line()	amohapatra	Feb. 13, 2021, 2:53 p.m.
This should be self.stdout.new_line()	amohapatra	Feb. 13, 2021, 2:52 p.m.
This should be self.stdout.new_line()	amohapatra	Feb. 13, 2021, 2:52 p.m.
This should be self.stdout.new_line()	amohapatra	Feb. 13, 2021, 2:52 p.m.
This should be self.stdout.new_line()	amohapatra	Feb. 13, 2021, 2:52 p.m.
W292 no newline at end of file	reviewbot	Feb. 13, 2021, 2:59 p.m.
W391 blank line at end of file	reviewbot	Feb. 13, 2021, 3:06 p.m.
W293 blank line contains whitespace	reviewbot	Feb. 13, 2021, 3:06 p.m.
E501 line too long (81 > 79 characters)	reviewbot	Feb. 19, 2021, 11:32 a.m.
E501 line too long (89 > 79 characters)	reviewbot	Feb. 19, 2021, 11:32 a.m.
E501 line too long (83 > 79 characters)	reviewbot	Feb. 19, 2021, 11:32 a.m.
E501 line too long (89 > 79 characters)	reviewbot	Feb. 19, 2021, 11:32 a.m.
All test cases should be in files called test_X, not the __init__.py. In this case I think you could probably ...	david	March 2, 2021, 8:56 p.m.
A few changes in here: The file needs a module docstring The first thing after that docstring should be from ...	david	March 5, 2021, 7:26 p.m.
Dedent this three spaces.	david	March 2, 2021, 8:58 p.m.
Typo: strea -> stream	david	March 2, 2021, 9 p.m.
This needs a method docstring.	david	March 27, 2021, 12:19 p.m.
This needs a method docstring.	david	March 27, 2021, 12:21 p.m.
This needs a method docstring.	david	March 27, 2021, 12:21 p.m.
Can we call these _bytes instead of _byte?	david	March 27, 2021, 12:24 p.m.
Let's add another newline in here, before the format operation. Otherwise it's sometimes easy to miss when scanning quickly through ...	david	March 27, 2021, 12:26 p.m.
The second line here needs to be indented a bit more: % (review_request.submitter.fullname or review_request.submitter.username))	david	March 27, 2021, 12:26 p.m.
When we wrap a string, we put the space at the end of the first line. Let's also put the ...	david	March 27, 2021, 12:27 p.m.
Please undo the changes here (so the space is at the end of the first line).	david	March 27, 2021, 12:28 p.m.
One thing Christian just pointed out to me is that he's been moving to just import kgb, and then using ...	david	March 27, 2021, 12:30 p.m.
Please make sure this is sorted alphabetically.	david	March 27, 2021, 12:31 p.m.
We still need two blank lines here.	david	March 27, 2021, 1:04 p.m.
This isn't a good description. Let's say "Unit tests for command OutputWrapper"	david	March 27, 2021, 1:05 p.m.
Docstrings need to be in the following form: """Single-line summary. Multi-line description. """ Along with this, for any newly-introduced classes ...	chipx86	April 2, 2021, 12:08 p.m.
Function docstrings must follow the same form, and specify any arguments, return values, and exceptions raised. Take a look at ...	chipx86	April 2, 2021, 12:43 p.m.
We expect a blank line between the end of a block and a new statememt, to help separate that out ...	chipx86	April 2, 2021, 12:52 p.m.
There's some typos in these docstrings ("charater" here, "specifiy" in the class docstring). If your editor supports it, I recommend ...	chipx86	April 2, 2021, 12:53 p.m.
This docstring is old, and needs to be modernized. As a step toward this, since you're introducing some really important ...	chipx86	April 3, 2021, 10:09 a.m.
There's an inconsistency in variable names. You have stderr_bytes (plural) and stderr_byte (singular). The plural form is what we want, ...	chipx86	April 3, 2021, 10:10 a.m.
Here's an area where the new output wrappers solve a problem! Not all diffs can be decoded as UTF-8, so ...	chipx86	April 3, 2021, 11:37 a.m.
The string is a parameter to textwrap.fill(), rather than a second parameter to stdout.write. Since we're wrapping to the following ...	chipx86	April 3, 2021, 11:43 a.m.
To keep lint checkers from complaining about indentations being multiples of 4, this should actually be in the same form ...	chipx86	April 3, 2021, 11:53 a.m.
This should be a single import statement.	chipx86	April 3, 2021, 11:54 a.m.
This is a class docstring, so it needs to have a trailing period.	chipx86	April 3, 2021, 11:56 a.m.
All unit test docstrings should be in the form of Testing <Thing> <condition>. For instance, Testing OutputWrapper initializes stream Testing ...	chipx86	April 3, 2021, 12:13 p.m.
W293 blank line contains whitespace	reviewbot	April 3, 2021, 12:23 p.m.
W293 blank line contains whitespace	reviewbot	April 3, 2021, 12:23 p.m.
W293 blank line contains whitespace	reviewbot	April 3, 2021, 12:24 p.m.
E501 line too long (80 > 79 characters)	reviewbot	April 3, 2021, 12:24 p.m.
W293 blank line contains whitespace	reviewbot	April 3, 2021, 12:24 p.m.
W293 blank line contains whitespace	reviewbot	April 3, 2021, 12:24 p.m.
E126 continuation line over-indented for hanging indent	reviewbot	April 3, 2021, 12:30 p.m.
F401 'rbtools.commands.JSONWrapper' imported but unused	reviewbot	April 10, 2021, 6:17 p.m.
F821 undefined name 'JSONOutput'	reviewbot	April 10, 2021, 6:17 p.m.

flake8 failed.

JSHint passed.

flake8

rbtools/commands/__init__.py (Diff revision 1)
Show all issues
```
E501 line too long (85 > 79 characters)
```
rbtools/commands/__init__.py (Diff revision 1)
Show all issues
```
E501 line too long (91 > 79 characters)
```
rbtools/commands/__init__.py (Diff revision 1)
Show all issues
```
W293 blank line contains whitespace
```
rbtools/commands/alias.py (Diff revision 1)
Show all issues
```
E501 line too long (85 > 79 characters)
```
rbtools/commands/alias.py (Diff revision 1)
Show all issues
```
E501 line too long (88 > 79 characters)
```
rbtools/commands/alias.py (Diff revision 1)
Show all issues
```
E501 line too long (89 > 79 characters)
```
rbtools/commands/info.py (Diff revision 1)
Show all issues
```
E211 whitespace before '('
```
rbtools/commands/land.py (Diff revision 1)
Show all issues
```
E501 line too long (91 > 79 characters)
```
rbtools/commands/land.py (Diff revision 1)
Show all issues
```
E501 line too long (90 > 79 characters)
```
rbtools/commands/patch.py (Diff revision 1)
Show all issues
```
E501 line too long (89 > 79 characters)
```
rbtools/commands/patch.py (Diff revision 1)
Show all issues
```
E501 line too long (88 > 79 characters)
```
rbtools/commands/patch.py (Diff revision 1)
Show all issues
```
E501 line too long (85 > 79 characters)
```
rbtools/commands/patch.py (Diff revision 1)
Show all issues
```
E501 line too long (84 > 79 characters)
```
rbtools/commands/post.py (Diff revision 1)
Show all issues
```
E501 line too long (86 > 79 characters)
```
rbtools/commands/post.py (Diff revision 1)
Show all issues
```
E501 line too long (83 > 79 characters)
```
rbtools/commands/post.py (Diff revision 1)
Show all issues
```
E501 line too long (84 > 79 characters)
```
rbtools/commands/post.py (Diff revision 1)
Show all issues
```
E501 line too long (80 > 79 characters)
```
rbtools/commands/post.py (Diff revision 1)
Show all issues
```
E501 line too long (91 > 79 characters)
```
rbtools/commands/publish.py (Diff revision 1)
Show all issues
```
E501 line too long (81 > 79 characters)
```
rbtools/commands/setup_completion.py (Diff revision 1)
Show all issues
```
E501 line too long (80 > 79 characters)
```
rbtools/commands/setup_repo.py (Diff revision 1)
Show all issues
```
E501 line too long (83 > 79 characters)
```
rbtools/commands/setup_repo.py (Diff revision 1)
Show all issues
```
E501 line too long (88 > 79 characters)
```
rbtools/commands/setup_repo.py (Diff revision 1)
Show all issues
```
E501 line too long (89 > 79 characters)
```
rbtools/commands/tests/__init__.py (Diff revision 1)
Show all issues
```
E501 line too long (80 > 79 characters)
```
rbtools/commands/tests/__init__.py (Diff revision 1)
Show all issues
```
W292 no newline at end of file
```

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
+	[WIP] 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

Description:

~		[WIP] RBTools commands were previously using `print` and `sys.stderr`
	~	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.

Groups:

+	students

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

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/commands/__init__.py (Diff revision 2)
Show all issues
```
W291 trailing whitespace
```

rbtools/commands/land.py (Diff revision 2)

Show all issues

E124 closing bracket does not match visual indentation

rbtools/commands/setup_repo.py (Diff revision 2)
Show all issues
```
E231 missing whitespace after ','
```
rbtools/commands/status_update.py (Diff revision 2)
Show all issues
```
E501 line too long (86 > 79 characters)
```
rbtools/commands/tests/__init__.py (Diff revision 2)
Show all issues
```
W291 trailing whitespace
```

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
+	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

Diff:

Revision 3 (+506 -352)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

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

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/commands/tests/__init__.py (Diff revision 4)
Show all issues
```
W292 no newline at end of file
```

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

Show all issues

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

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

Show all issues

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

rbtools/commands/setup_repo.py (Diff revision 4)
Show all issues
```
This should be self.stdout.new_line()?
```
rbtools/commands/setup_repo.py (Diff revision 4)
Show all issues
```
This should be self.stdout.new_line() ?
```
rbtools/commands/setup_repo.py (Diff revision 4)
Show all issues
```
This should also be self.stdout.new_line()
```
rbtools/commands/setup_repo.py (Diff revision 4)
Show all issues
```
This should be self.stdout.new_line()
```
rbtools/commands/setup_repo.py (Diff revision 4)
Show all issues
```
This should be self.stdout.new_line()
```
rbtools/commands/setup_repo.py (Diff revision 4)
Show all issues
```
This should be self.stdout.new_line()
```
rbtools/commands/setup_repo.py (Diff revision 4)
Show all issues
```
This should be self.stdout.new_line()
```
rbtools/commands/setup_repo.py (Diff revision 4)
Show all issues
```
This should be self.stdout.new_line()
```
rbtools/commands/setup_repo.py (Diff revision 4)
Show all issues
```
This should be self.stdout.new_line()
```
rbtools/commands/setup_repo.py (Diff revision 4)
Show all issues
```
This should be self.stdout.new_line()
```

Show all issues

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

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

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/commands/tests/__init__.py (Diff revision 5)
Show all issues
```
W391 blank line at end of file
```
rbtools/commands/tests/__init__.py (Diff revision 5)
Show all issues
```
W293 blank line contains whitespace
```

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()
+	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

Diff:

Revision 6 (+542 -370)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Changed status from work in progress to in review

Summary:

-	[WIP] 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
+	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

Testing Done:

~		Ran all tests in ./tests/runtests.py rbtools/commands/ and passed.
	~	Ran all tests in `./tests/runtests.py rbtools.commands` and passed.

~		Added two new tests for `__init.py`. One that tests if output stream
	~	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

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

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/commands/__init__.py (Diff revision 7)
Show all issues
```
E501 line too long (81 > 79 characters)
```
rbtools/commands/__init__.py (Diff revision 7)
Show all issues
```
E501 line too long (89 > 79 characters)
```
rbtools/commands/__init__.py (Diff revision 7)
Show all issues
```
E501 line too long (83 > 79 characters)
```
rbtools/commands/__init__.py (Diff revision 7)
Show all issues
```
E501 line too long (89 > 79 characters)
```

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
+	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

Diff:

Revision 8 (+579 -405)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

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

Show all issues

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.

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

Show all issues

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

rbtools/commands/tests/__init__.py (Diff revision 8)
Show all issues
```
Dedent this three spaces.
```
rbtools/commands/tests/__init__.py (Diff revision 8)
Show all issues
```
Typo: strea -> stream
```

Show all issues

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

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
+	OutputWrapper class to Commands which outputs to stream object of choice

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
+	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

Diff:

Revision 9 (+615 -449)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

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
+	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

Diff:

Revision 10 (+617 -449)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/commands/__init__.py (Diff revision 10)
Show all issues
```
This needs a method docstring.
```
rbtools/commands/__init__.py (Diff revision 10)
Show all issues
```
This needs a method docstring.
```
rbtools/commands/__init__.py (Diff revision 10)
Show all issues
```
This needs a method docstring.
```
rbtools/commands/__init__.py (Diff revision 10)
Show all issues
```
Can we call these _bytes instead of _byte?
```

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

Show all issues

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])

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

Show all issues

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

% (review_request.submitter.fullname or
   review_request.submitter.username))

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

Show all issues

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)

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

Show all issues

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

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

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

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

Show all issues

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.

rbtools/commands/tests/test_main.py (Diff revision 10)
Show all issues
```
Please make sure this is sorted alphabetically.
```
rbtools/commands/tests/test_main.py (Diff revision 10)
Show all issues
```
We still need two blank lines here.
```

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

Show all issues

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

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 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

Diff:

Revision 11 (+652 -458)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

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

Show all issues

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
"""

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

Show all issues

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.

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

Show all issues

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.

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

Show all issues

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.

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

Show all issues

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.

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

Show all issues

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.

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

Show all issues

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.

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

Show all issues

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.

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

Show all issues

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.

rbtools/commands/tests/test_main.py (Diff revision 11)
Show all issues
```
This should be a single import statement.
```

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

Show all issues

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

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

Show all issues

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.

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

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/commands/__init__.py (Diff revision 12)
Show all issues
```
W293 blank line contains whitespace
```
rbtools/commands/__init__.py (Diff revision 12)
Show all issues
```
W293 blank line contains whitespace
```
rbtools/commands/__init__.py (Diff revision 12)
Show all issues
```
W293 blank line contains whitespace
```
rbtools/commands/__init__.py (Diff revision 12)
Show all issues
```
E501 line too long (80 > 79 characters)
```
rbtools/commands/__init__.py (Diff revision 12)
Show all issues
```
W293 blank line contains whitespace
```
rbtools/commands/__init__.py (Diff revision 12)
Show all issues
```
W293 blank line contains whitespace
```

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

Show all issues

E126 continuation line over-indented for hanging indent

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.
+		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

Diff:

Revision 13 (+765 -515)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

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

	rbtools/commands/__init__.py
	rbtools/commands/tests/test_main.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

rbtools/commands/tests/test_main.py (Diff revision 14)
Show all issues
```
F401 'rbtools.commands.JSONWrapper' imported but unused
```
rbtools/commands/tests/test_main.py (Diff revision 14)
Show all issues
```
F821 undefined name 'JSONOutput'
```

Commits:

	Summary
-		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
+		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
+		Fixed bug where test_main.py was imported JSONWrapper instead of JSONOutput

Diff:

Revision 15 (+466 -100)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/tests/test_main.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commits:

	Summary
-		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
-		Fixed bug where test_main.py was imported JSONWrapper instead of JSONOutput
+		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

Diff:

Revision 16 (+765 -515)

Show changes

	rbtools/commands/__init__.py
	rbtools/commands/alias.py
	rbtools/commands/api_get.py
	rbtools/commands/attach.py
	rbtools/commands/close.py
	rbtools/commands/diff.py
	rbtools/commands/info.py
	rbtools/commands/land.py
	9 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

Making some tweaks and getting this landed. Thanks!

Status: Closed (submitted)

Change Summary:

Pushed to master (db0c1f5)

You have a pending review.

OutputWrapper class to Commands which outputs to stream object of choice

flake8

Summary:

Description:

Groups:

Change Summary:

Commits:

Diff:

Checks run (1 failed, 1 succeeded)

flake8

Commits:

Diff:

Checks run (2 succeeded)

Change Summary:

Commits:

Diff:

Checks run (1 failed, 1 succeeded)

flake8

Change Summary:

Commits:

Diff:

Checks run (1 failed, 1 succeeded)

flake8

Commits:

Diff:

Checks run (2 succeeded)

Change Summary:

Summary:

Testing Done:

Commits:

Diff:

Checks run (1 failed, 1 succeeded)

flake8

Commits:

Diff:

Checks run (2 succeeded)

Summary:

Commits:

Diff:

Checks run (2 succeeded)

Commits:

Diff:

Checks run (2 succeeded)

Commits:

Diff:

Checks run (2 succeeded)

Commits:

Diff:

Checks run (1 failed, 1 succeeded)

flake8

Commits:

Diff:

Checks run (2 succeeded)

Commits:

Diff:

Checks run (1 failed, 1 succeeded)

flake8

Commits:

Diff:

Checks run (2 succeeded)

Commits:

Diff:

Checks run (2 succeeded)

Status: Closed (submitted)

Change Summary: