Summary

Fix various PEP 8, style and PyFlakes issues

Review Request #9655 — Created Feb. 16, 2018 and submitted April 18, 2018, 5:39 p.m.

Information

Owner

solarmist

Repository

RBTools

Branch

master

Bugs

Depends On

~~9654~~

Blocks

9648

Commit

6540441...

Reviewers

Groups

rbtools

People

Description

Fix various PEP 8, sytle and PyFlakes issues.

I've also included an exception for E129 in the setup.cfg to make it explicit in the codebase
that this is the expected style.

Testing Done

(%) flake8 -v tests rbtools
flake8.checker            MainProcess    409 INFO     Making checkers

flake8.checker            MainProcess    650 INFO     Checking 79 files

flake8.main.application   MainProcess   2483 INFO     Finished running

flake8.main.application   MainProcess   2484 INFO     Reporting errors

flake8.main.application   MainProcess   2485 INFO     Found a total of 37 violations and reported 0
Note: These violations were in appdirs.py and the docs directory.
time ./tests/runtests.py  

/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/pyversion.py:36: DeprecationWarning: The 'new' module has been removed in Python 3.0; use the 'types' module instead.

  import new

/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/plugins/prof.py:14: DeprecationWarning: The 'hotshot' module is not supported in 3.x, use the 'profile' module instead.

  import hotshot

/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/pyversion.py:49: DeprecationWarning: Overriding __eq__ blocks inheritance of __hash__ in 3.x

  class Key(object):

#1 Testing the cache with a high max-age value ... /Volumes/Augie/solarmist/rbtools/rbtools/api/cache.py:479: DeprecationWarning: buffer() not supported in 3.x

  sqlite3.Binary(entry.response_body)))

ok

#2 Testing the cache with a zero max-age value ... /Volumes/Augie/solarmist/rbtools/rbtools/api/cache.py:493: DeprecationWarning: buffer() not supported in 3.x

  sqlite3.Binary(entry.response_body), entry.url,

ok
Ran 210 tests in 215.007s
OK

./tests/runtests.py  93.50s user 60.71s system 71% cpu 3:35.47 total

Issues

Description	From	Last Updated
Something else to mention is that, as proof that these changes don't regress Python 2.7 and that they work on …	chipx86	Feb. 17, 2018, 10:17 a.m.
We have a certain way we like to see content for review request summaries, descriptions, and Testing Done. The descriptions …	chipx86	Feb. 17, 2018, 10:25 a.m.
You really don't need the entire output of the tests in the testing done. You can just say you ran …	brennie	Feb. 22, 2018, 3:40 p.m.
Can you revert this? In cases like this, we prefer the variable on its own line, to give room for …	chipx86	Feb. 17, 2018, 9:12 a.m.
Same here.	chipx86	Feb. 17, 2018, 9:12 a.m.
We ignore the pep8 tool here. These should always be aligned, no indentation.	chipx86	Feb. 17, 2018, 9:12 a.m.
No indentation here, either.	chipx86	Feb. 17, 2018, 9:12 a.m.
Nor here.	chipx86	Feb. 17, 2018, 9:12 a.m.
Nor here.	chipx86	Feb. 17, 2018, 9:12 a.m.
We actually shouldn't have a period at the end of this, since it's for a unit test. Remove that and …	chipx86	Feb. 17, 2018, 9:12 a.m.
No indentation.	chipx86	Feb. 17, 2018, 9:11 a.m.
Nor here.	chipx86	Feb. 17, 2018, 9:11 a.m.
Nor these. Mind going through the rest of the change and undoing these? It will help. I don't want to …	chipx86	Feb. 17, 2018, 9:11 a.m.
This can be changed to: if sys.platform.startswith(('win', 'cygwin')):	chipx86	Feb. 17, 2018, 9:15 a.m.
In cases like this, we prefer the % on the following line, like: raise SCMError('...' % var)	chipx86	Feb. 17, 2018, 9:15 a.m.
This should be undone. Logging arguments on the line after the string.	chipx86	Feb. 17, 2018, 9:15 a.m.
The old way was preferable.	chipx86	Feb. 17, 2018, 9:18 a.m.
Here, too.	chipx86	Feb. 17, 2018, 9:18 a.m.
We prefer the old format.	chipx86	Feb. 17, 2018, 9:18 a.m.
Shouldn't be indented.	chipx86	Feb. 17, 2018, 9:18 a.m.
Can you rever this? We spread out our dictionaries, so that further changes don't affect as much code.	chipx86	Feb. 17, 2018, 9:19 a.m.
Each line should have the r prefix.	chipx86	Feb. 17, 2018, 9:21 a.m.
I'm not sure this is no longer the case, for Python 2 support. We needed this to fix a bug. …	chipx86	Feb. 17, 2018, 10:33 p.m.
While split across more lines, the previous version was fine. Easier to scan.	chipx86	Feb. 17, 2018, 9:44 a.m.
Please revert these. Long lines are fine if they're a URL, since we want them to be proper links. In …	chipx86	Feb. 17, 2018, 9:44 a.m.
Individual test case docstrings should not end in a period (the ones that are "Testing XXXX"), because the test runner …	david	Feb. 17, 2018, 10:34 p.m.
Add a blank line above this one.	david	Feb. 17, 2018, 10:36 p.m.
Period here.	david	Feb. 17, 2018, 10:34 p.m.
Period here.	david	Feb. 17, 2018, 10:34 p.m.
Period here.	david	Feb. 17, 2018, 10:35 p.m.
Period here.	david	Feb. 17, 2018, 10:35 p.m.
Period here.	david	Feb. 17, 2018, 10:44 p.m.
Add a blank line above this one.	david	Feb. 17, 2018, 10:44 p.m.
Period here.	david	Feb. 17, 2018, 10:44 p.m.
Period here.	david	Feb. 17, 2018, 10:44 p.m.
Period here.	david	Feb. 17, 2018, 10:44 p.m.
Period here.	david	Feb. 17, 2018, 10:44 p.m.
Revert this change.	david	Feb. 17, 2018, 10:44 p.m.
Revert this change.	david	Feb. 17, 2018, 10:44 p.m.
Revert this change.	david	Feb. 17, 2018, 10:44 p.m.
Revert this change.	david	Feb. 17, 2018, 10:44 p.m.
Revert this change.	david	Feb. 17, 2018, 10:44 p.m.
Revert this change.	david	Feb. 17, 2018, 10:44 p.m.
Revert this change.	david	Feb. 17, 2018, 10:44 p.m.
Revert this change.	david	Feb. 17, 2018, 10:45 p.m.
This is copy/pasted output from the command. I don't think we should change it.	david	Feb. 17, 2018, 3:30 p.m.
This is copy/pasted output from the command. I don't think we should change it.	david	Feb. 17, 2018, 3:30 p.m.
Revert this change.	david	Feb. 17, 2018, 10:45 p.m.
Revert this change.	david	Feb. 17, 2018, 10:45 p.m.
Revert this change.	david	Feb. 17, 2018, 10:45 p.m.
Revert this change.	david	Feb. 17, 2018, 10:45 p.m.
Revert this change.	david	Feb. 17, 2018, 10:45 p.m.
Revert this change.	david	Feb. 17, 2018, 10:45 p.m.
Revert this change.	david	Feb. 17, 2018, 10:45 p.m.
Revert this change.	david	Feb. 17, 2018, 10:45 p.m.
Revert this change.	david	Feb. 17, 2018, 10:45 p.m.
This is a wildcard and so should be under the list of wildcards.	brennie	Feb. 22, 2018, 7:27 p.m.
Instead, how about: content.write(b'%s%s' % (six.text_type(self._fields[key]).encode('utf-8'), NEWLINE)	brennie	Feb. 22, 2018, 3:44 p.m.
Instead, how about: content.write(b'filename="%s"%s' % (filename.encode('utf-8'), NEWLINE))	brennie	Feb. 22, 2018, 3:44 p.m.
The way this is wrapped is now more confusing. How about: content.write( six.text_type(self._fields[key]).encode('utf-8') + NEWLINE)	david	March 16, 2018, 7:10 a.m.
To be consistent with our other configurations, let's do E121,E125,E129,E241	david	March 16, 2018, 7 a.m.
This still isn't how I suggested you wrap it. content.write( six.text_type(self._fields[key]).encode('utf-8') + NEWLINE)	david	March 24, 2018, 2:45 p.m.
This can be a little bit more compact: if (git_top.startswith(('fatal:', 'cygdrive')) or not os.path.isdir(git_dir)):	david	March 24, 2018, 2:50 p.m.
Extra line here isn't necessary unless there's a class docstring.	david	March 24, 2018, 2:55 p.m.

flake8 passed.

JSHint passed.

Depends On:

+	~~9648 - Fix exception handling, and update imports~~
+	~~9649 - Make six use consistant.~~
+	~~9650 - Fix log messages~~
+	~~9651 - Make quote use consistent across repo~~
+	~~9652 - Fix various things for python 3 compatibility~~
+	~~9653 - Refactor encode_multipart_formdata and tests to be easier to read and maintain~~
+	~~9654 - Fix unit tests to not pollute the test directories of other tests.~~
+	~~9655 - Fix various PEP 8, style and PyFlakes issues~~

Hi,

Thanks for the contributions! David's been working on Python 3 support, with changes coming, so I'm not sure how much of these might collide, but it looks like there's a lot of great cleanup and improvements for some of the very old code in this codebase that I'd be glad to incorporate.

I want to get a sense of whether we should expect further changes as part of this series, or if this is the complete set, to avoid redundant work. We might want to coordinate more over e-mail, just in case.

There's a lot in this particular change that fix complaints from the pep8/pycodestyle warnings, but they are not things we choose to follow in our codebase. For example, you'll see a handful of comments about changes like:

if (a and
        b):

We don't follow this style, and Review Bot doesn't warn about it on our server. We prefer to keep things aligned. There's a handful of other changes like that in this review request that we'll want to see some tweaks to, and some we'll have you outright remove. If you wouldn't mind making similar changes to the other review requests, that will definitely help us review and incorporate these into the codebase. Hope you don't mind, and know we appreciate the work that went into this :)

solarmist Feb. 17, 2018, 10:17 a.m.

This should be the only "bulk" change and everything moving forward would be fixing individual bugs or adding test coverage where it is lacking.

The issue has been dropped. Show all issues

We have a certain way we like to see content for review request summaries, descriptions, and Testing Done. The descriptions don't need to reference other review requests (the Depends On field is enough for this), and instead should focus solely on what this change is about. We typically use a format describing, at a medium-to-high level, the problem being addressed by a change and what was done to fix it.

A guide and examples are here: https://www.notion.so/reviewboard/Writing-Good-Change-Descriptions-10529e7c207743fa8ca90153d4b21fea

Mind updating the review requests to fit that format?

solarmist Feb. 17, 2018, 10:25 a.m.

Sure, I was a bit unsure about it since this was one very large change broken down into pieces.

Updated for this review. I'm dropping this issue and will update each review accordingly as I fix issues.

rbtools/api/cache.py (Diff revision 1)

The issue has been resolved. Show all issues

Can you revert this? In cases like this, we prefer the variable on its own line, to give room for expanding the string as necessary. Basically, when there's multiple lines of arguments, we tend to put each on its own line.

solarmist Feb. 17, 2018, 10:17 a.m.
```
Sure.
```

rbtools/api/cache.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same here.
```

rbtools/api/request.py (Diff revision 1)

The issue has been resolved. Show all issues

We ignore the pep8 tool here. These should always be aligned, no indentation.

rbtools/api/resource.py (Diff revision 1)
The issue has been resolved. Show all issues
```
No indentation here, either.
```
rbtools/api/resource.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Nor here.
```
rbtools/api/resource.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Nor here.
```

rbtools/api/tests.py (Diff revision 1)

The issue has been resolved. Show all issues

We actually shouldn't have a period at the end of this, since it's for a unit test. Remove that and the """ will fit on the previous line.

rbtools/clients/__init__.py (Diff revision 1)
The issue has been resolved. Show all issues
```
No indentation.
```
rbtools/clients/__init__.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Nor here.
```

rbtools/clients/__init__.py (Diff revision 1)

The issue has been resolved. Show all issues

Nor these.

Mind going through the rest of the change and undoing these? It will help. I don't want to flood you with comments :)

rbtools/clients/clearcase.py (Diff revision 1)

The issue has been resolved. Show all issues

This can be changed to:

if sys.platform.startswith(('win', 'cygwin')):

solarmist Feb. 17, 2018, 10:17 a.m.
```
That's handy. I wasn't aware of that.
```

rbtools/clients/perforce.py (Diff revision 1)

The issue has been resolved. Show all issues

In cases like this, we prefer the % on the following line, like:

raise SCMError('...'
               % var)

rbtools/clients/perforce.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This should be undone. Logging arguments on the line after the string.
```
rbtools/clients/plastic.py (Diff revision 1)
The issue has been resolved. Show all issues
```
The old way was preferable.
```
rbtools/clients/plastic.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Here, too.
```
rbtools/clients/svn.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We prefer the old format.
```
rbtools/clients/svn.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Shouldn't be indented.
```

rbtools/clients/tests/test_svn.py (Diff revision 1)

The issue has been resolved. Show all issues

Can you rever this? We spread out our dictionaries, so that further changes don't affect as much code.

rbtools/clients/tfs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Each line should have the r prefix.
```

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

The issue has been dropped. Show all issues

I'm not sure this is no longer the case, for Python 2 support. We needed this to fix a bug.

If this is for Python 3 support, a better option would be to do:

RB_MAIN = str('rbt')

This ensures a byte string on Python 2, Unicode string on Python 3. Given that, the comment should remain, but be expanded.

solarmist Feb. 17, 2018, 10:17 a.m.

But can you explain more? I can't find any reference to this and I haven't seen any warnings or errors due to this.

chipx86 Feb. 17, 2018, 10:25 a.m.

On Python 2.x, we were hitting a problem where byte strings containing non-ASCII characters were being implicitly converted to Unicode strings due to being concatenated onto RB_MAIN (which was a Unicode string and was taking precedence due to being first).

The reason for str() has to do with the differences in default string types on Python 2 and 3. For Python 2, the default type is a byte string (and you have to opt-in to Unicode strings with either a u'...' or by doing from __future__ import unicode_literals. On Python 3, the default is a Unicode string, and you have to opt into a byte string with b'...'.

str(), in both versions, represents the default type string for the language (not the file). So for Python 2, it gives you byte strings. For Python 3, Unicode strings.

However, I'm not certain that Unicode strings on Python 3 are safe here. It might still trigger the same bug, because we're dealing with data from an unknown locale.

For now, I'd like to revert this change and instead see it fixed along with new unit tests that successfully initially reproduce the original problem on Python 2 and see if it (and a fix) impacts Python 3.

solarmist Feb. 17, 2018, 10:34 p.m.

Removed from this commit. I wasn't running into this at all in my testing. It probably has to do with the terminal LANG.

chipx86 Feb. 18, 2018, 12:27 a.m.

Yeah, I know the original repro case was from a Chinese user who was setting a default summary or description through the command line arguments (--summary, --description) to Chinese text, and I don't believe the encoding was UTF-8.

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

The issue has been resolved. Show all issues

While split across more lines, the previous version was fine. Easier to scan.

rbtools/utils/appdirs.py (Diff revision 1)

The issue has been resolved. Show all issues

Please revert these. Long lines are fine if they're a URL, since we want them to be proper links.

In fact, all changes to this file should be reverted. It tracks an upstream package, and we don't want to make changes to it, as it's harder to figure out the differences between versions this way.

solarmist Feb. 17, 2018, 10:17 a.m.
```
Done. Dropped for all style changes.
```

The issue has been dropped. Show all issues

Something else to mention is that, as proof that these changes don't regress Python 2.7 and that they work on Python 3.4, 3.5, and 3.6 (our targets), we'd like to see new unit tests for anything not currently covered. If you have questions on this, please reach out to us, as we know the test coverage in RBTools is not currently as comprehensive as most of our software.

solarmist Feb. 17, 2018, 10:17 a.m.

I assume this is referring to the other changes as this commit is purely a style change and flake8 would pickup any syntax errors.

chipx86 Feb. 17, 2018, 10:26 a.m.

This mostly refers to the other changes, but I just left a note about how the RB_MAIN change impacts things and the testing needed. Anything that is not purely style (anything changing string types, for instance) requires investigation into the need for a certain type and then unit tests around it.

solarmist Feb. 17, 2018, 3:39 p.m.

I will at a minimimum move this out of this change.

Description:

		Fix various PEP 8, sytle and PyFlakes issues.

~		This is the final change in the series.
~		https://reviews.reviewboard.org/r/9648/
	~	I've also included an exception for E129 in the setup.cfg to make it explicit in the codebase
	~	that this is the expected style.
-		https://reviews.reviewboard.org/r/9649/
-		https://reviews.reviewboard.org/r/9650/
-		https://reviews.reviewboard.org/r/9651/
-		https://reviews.reviewboard.org/r/9652/
-		https://reviews.reviewboard.org/r/9653/
-		https://reviews.reviewboard.org/r/9654/
-		https://reviews.reviewboard.org/r/9655/

Testing Done:

~		At this point flake8 and the unit tests should run and pass.
	~	(%) flake8 -v tests rbtools
	+
	+	flake8.checker MainProcess 409 INFO Making checkers
	+	flake8.checker MainProcess 650 INFO Checking 79 files
	+	flake8.main.application MainProcess 2483 INFO Finished running
	+	flake8.main.application MainProcess 2484 INFO Reporting errors
	+	flake8.main.application MainProcess 2485 INFO Found a total of 37 violations and reported 0
	+
	+	Note: I have all of the local changes to rbtools/utils/appdirs.py still, that's why there's no PEP8 issues for that file.

Commit:

-	141270ed1a694a51fb4fb3c05b90fdb943d87d1b
+	82f4c0109a81892cd95c2611c47bb2556775f934

Diff:

Revision 2 (+93 -89)

Show changes

	.gitignore
	setup.cfg
	rbtools/api/decode.py
	rbtools/api/request.py
	rbtools/api/tests.py
	rbtools/clients/clearcase.py
	rbtools/clients/git.py
	rbtools/clients/perforce.py
	13 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Depends On:

-	~~9655 - Fix various PEP 8, style and PyFlakes issues~~

rbtools/api/tests.py (Diff revision 2)

The issue has been resolved. Show all issues

Individual test case docstrings should not end in a period (the ones that are "Testing XXXX"), because the test runner will add an ellipsis. Other docstrings, like these. should have a period at the end.

solarmist Feb. 17, 2018, 3:38 p.m.

I'm fine with making these changes and while most people won't be attempting to make large scale changes to code like I am, I feel like this kind of rule adds unneeded sublty to docstring in unit test files.

I'd propose for simplicity that in test files the summary lines should exclude the period. Otherwise the documentation should be updated to call out this exception for test cases.

rbtools/api/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Add a blank line above this one.
```
1. solarmist Feb. 17, 2018, 3:38 p.m.
  Will do.
rbtools/api/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Period here.
```
rbtools/api/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Period here.
```
rbtools/api/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Period here.
```
rbtools/api/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Period here.
```
rbtools/api/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Period here.
```
rbtools/api/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Add a blank line above this one.
```
rbtools/api/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Period here.
```
rbtools/api/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Period here.
```
rbtools/api/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Period here.
```
rbtools/api/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Period here.
```
rbtools/clients/tests/__init__.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/__init__.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_bzr.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_bzr.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_bzr.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_bzr.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_git.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_git.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```

rbtools/clients/tests/test_git.py (Diff revision 2)

The issue has been dropped. Show all issues

This is copy/pasted output from the command. I don't think we should change it.

solarmist Feb. 17, 2018, 3:38 p.m.

I don't think materially changes the output, unless you expect someone to try an exact character match of this text.

I feel like it should be alright to make this kind of whitespace change.

rbtools/clients/tests/test_git.py (Diff revision 2)

The issue has been dropped. Show all issues

This is copy/pasted output from the command. I don't think we should change it.

solarmist Feb. 17, 2018, 3:38 p.m.

Again I feel like this change is immaterial to it's contents and except for a whitespace difference git itself swaps between this format and the one in the previous comment.

rbtools/clients/tests/test_mercurial.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_mercurial.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_mercurial.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_p4.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_p4.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_p4.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_svn.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_svn.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```
rbtools/clients/tests/test_svn.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Revert this change.
```

Summary:

-	Fix various PEP 8, sytle and PyFlakes issues
+	Fix various PEP 8, style and PyFlakes issues

Commit:

-	82f4c0109a81892cd95c2611c47bb2556775f934
+	c474fead6d21e5375e853b2ff768de5d1ae6a891

Diff:

Revision 3 (+67 -60)

Show changes

	.gitignore
	setup.cfg
	rbtools/api/decode.py
	rbtools/api/request.py
	rbtools/api/tests.py
	rbtools/clients/clearcase.py
	rbtools/clients/git.py
	rbtools/clients/perforce.py
	9 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Found a few additional spots missing blank lines.

Commit:

-	c474fead6d21e5375e853b2ff768de5d1ae6a891
+	a5ab6d3cb14ddc9007f96ea8c43008db9fc4393b

Diff:

Revision 4 (+75 -60)

Show changes

	.gitignore
	setup.cfg
	rbtools/api/decode.py
	rbtools/api/request.py
	rbtools/api/tests.py
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	rbtools/clients/git.py
	11 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

After updating the setup.cfg I was able to catch the issues in contrib

Testing Done:

		(%) flake8 -v tests rbtools

		flake8.checker MainProcess 409 INFO Making checkers
		flake8.checker MainProcess 650 INFO Checking 79 files
		flake8.main.application MainProcess 2483 INFO Finished running
		flake8.main.application MainProcess 2484 INFO Reporting errors
~		flake8.main.application MainProcess 2485 INFO Found a total of 37 violations and reported 0
	~	flake8.main.application MainProcess 2485 INFO Found a total of 37 violations and reported 0
	+	ime ./tests/runtests.py [git:f7e978d...??]??
	+	/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/pyversion.py:36: DeprecationWarning: The 'new' module has been removed in Python 3.0; use the 'types' module instead.
	+	import new
	+	/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/plugins/prof.py:14: DeprecationWarning: The 'hotshot' module is not supported in 3.x, use the 'profile' module instead.
	+	import hotshot
	+	/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/pyversion.py:49: DeprecationWarning: Overriding eq blocks inheritance of hash in 3.x
	+	class Key(object):

~		Note: I have all of the local changes to rbtools/utils/appdirs.py still, that's why there's no PEP8 issues for that file.
	~	1 Testing the cache with a high max-age value ... /Volumes/Augie/solarmist/rbtools/rbtools/api/cache.py:479: DeprecationWarning: buffer() not supported in 3.x
	+
	+	sqlite3.Binary(entry.response_body)))
	+	ok
	+
	+	2 Testing the cache with a zero max-age value ... /Volumes/Augie/solarmist/rbtools/rbtools/api/cache.py:493: DeprecationWarning: buffer() not supported in 3.x
	+
	+	sqlite3.Binary(entry.response_body), entry.url,
	+	ok
	+
	+	3 Testing the cache with the must-revalidate control ... ok
	+
	+	4 Testing the cache with the no-store control ... ok
	+
	+	5 Testing the cache with the no-cache control ... ok
	+
	+	6 Testing the cache with the no-cache control and a specified ETag ... ok
	+
	+	7 Testing the cache with the no-cache control and an updated ETag ... ok
	+
	+	8 Testing the cache with the no-cache control ... ok
	+
	+	9 Testing the cache with the no-cache control and an updated Last-Modified header ... ok
	+
	+	10 Testing the cache with the Expires header in the past ... ok
	+
	+	11 Testing the cache with the Expires header in the future ... ok
	+
	+	12 Testing the cache with an Expires header that is overridden ... ok
	+
	+	13 Testing the cache with the Pragma: no-cache header ... ok
	+
	+	14 Testing writing to the cache with non-ASCII data ... ok
	+
	+	15 Testing the cache with the Vary header ... ok
	+
	+	16 Testing the cache with the Vary header and different requests ... ok
	+
	+	17 Testing Capabilities.has_capability with supported capability ... ok
	+
	+	18 Testing Capabilities.has_capability with partial capability path ... ok
	+
	+	19 Testing Capabilities.has_capability with unknown capability ... ok
	+
	+	20 Testing the default values ... ok
	+
	+	21 Testing the multipart form data generation ... ok
	+
	+	22 Testing the encoding of multipart form data with unicode and binary field data ... ok
	+
	+	23 Testing constructing a count resource ... ok
	+
	+	24 Testing constructing an item resource ... ok
	+
	+	25 Testing constructing a list resource ... ok
	+
	+	26 Testing constructing without guessing the resource token ... ok
	+
	+	27 Testing constructing a resource with a specific base class ... ok
	+
	+	28 Testing guessing the resource's token ... ok
	+
	+	29 Testing rewriting of extra_data__ parameters to create ... ok
	+
	+	30 Testing rewriting of exta_data__ parameters to update ... ok
	+
	+	31 Testing item resource fields ... ok
	+
	+	32 Testing item resource link generation ... ok
	+
	+	33 Testing access of a link field ... ok
	+
	+	34 Testing link resource link generation ... ok
	+
	+	35 Testing list resource lists ... ok
	+
	+	36 Testing access of a dictionary field ... ok
	+
	+	37 Testing iterating sub-fields of a dictionary field ... ok
	+
	+	38 Testing generation of methods for the root resource uri templates ... ok
	+
	+	39 Testing ReviewRequestResource.absolute_url with 'absolute_url' field ... ok
	+
	+	40 Testing ReviewRequestResource.absolute_url with generated fallback URL ... ok
	+
	+	41 Testing ReviewRequestResource.absolute_url with 'url' field ... ok
	+
	+	42 Testing BazaarClient diff with file exclusion ... ok
	+
	+	43 Testing BazaarClient diff with file exclusion in a subdirectory ... ok
	+
	+	44 Testing BazaarClient diff with file exclusion in the repo root ... ok
	+
	+	45 Testing BazaarClient diff with changes between a 2nd level descendant ... ok
	+
	+	46 Testing BazaarClient diff with changes only in the parent branch ... ok
	+
	+	47 Testing BazaarClient simple diff case ... ok
	+
	+	48 Testing BazaarClient simple diff with multiple commits case ... ok
	+
	+	49 Testing BazaarClient diff with specific files ... ok
	+
	+	50 Testing BazaarClient get_repository_info with child branch ... ok
	+
	+	51 Testing BazaarClient get_repository_info, no branch ... ok
	+
	+	52 Testing BazaarClient get_repository_info with original branch ... ok
	+
	+	53 Testing BazaarClient guessing summary and description ... ok
	+
	+	54 Testing BazaarClient guessing summary and description for grand parent branch ... ok
	+
	+	55 Testing BazaarClient guessing summary and description with a revision range ... ok
	+
	+	56 Testing BazaarClient.parse_revision_spec with no specified revisions ... ok
	+
	+	57 Testing BazaarClient.parse_revision_spec with one specified revision ... ok
	+
	+	58 Testing BazaarClient.parse_revision_spec with one specified revision and a parent diff ... ok
	+
	+	59 Testing BazaarClient.parse_revision_spec with R1..R2 syntax ... ok
	+
	+	60 Testing BazaarClient.parse_revision_spec with two revisions ... ok
	+
	+	61 Testing BazaarClient parse_revision_spec with too many revisions ... ok
	+
	+	62 Testing GitClient.create_commit with all_files set to True. ... ok
	+
	+	63 Testing GitClient.create_commit with run_editor set to True. ... ok
	+
	+	64 Testing GitClient.create_commit with all_files set to False. ... ok
	+
	+	65 Testing GitClient.create_commit with run_editor set to False. ... ok
	+
	+	66 Testing GitClient.delete_branch with merged_only set to True. ... ok
	+
	+	67 Testing GitClient.delete_branch with merged_only set to False. ... ok
	+
	+	68 Testing GitClient diff with divergent branches ... ok
	+
	+	69 Testing GitClient simple diff with file exclusion ... ok
	+
	+	70 Testing GitClient simple diff with file exclusion in a subdir ... ok
	+
	+	71 Testing GitClient diff with file exclusion in the repo root ... ok
	+
	+	72 Testing GitClient.parse_revision_spec with target branch off a tracking branch not aligned with the remote ... ok
	+
	+	73 Testing GitClient.parse_revision_spec with a target branch not up-to-date with a remote branch ... ok
	+
	+	74 Testing GitClient.parse_revision_spec with a target branch posted off a tracking branch that merged another tracking branch ... ok
	+
	+	75 Testing GitClient.parse_revision_spec with a target branch that merged a tracking branch off another tracking branch ... ok
	+
	+	76 Testing GitClient.parse_revision_spec with a target branch that has branches from different remotes in its path ... ok
	+
	+	77 Testing GitClient.parse_revision_spec with target branch off a tracking branch aligned with the remote ... ok
	+
	+	78 Testing GitClient.parse_revision_spec with a target branch posted off a remote branch that is aligned to the same commit as another remote branch ... ok
	+
	+	79 Testing GitClient.parse_revision_spec with a target branch posted off a remote branch without any tracking branches ... ok
	+
	+	80 Testing GitClient.parse_revision_spec with target branch off a branch not properly tracking the remote ... ok
	+
	+	81 Testing GitClient.parse_revision_spec with target branch off a tracking branch with changes since the remote ... ok
	+
	+	82 Testing GitClient diff with a local tracking branch ... ok
	+
	+	83 Testing GitClient simple diff case ... ok
	+
	+	84 Testing GitClient simple diff with multiple commits case ... ok
	+
	+	85 Testing GitClient diff with tracking branch that has slash in its name ... ok
	+
	+	86 Testing GitClient diff with a tracking branch, but no origin remote ... ok
	+
	+	87 Testing GitClient diff with option override for tracking branch ... ok
	+
	+	88 Testing GitClient.get_raw_commit_message ... ok
	+
	+	89 Testing GitClient get_repository_info, simple case ... ok
	+
	+	90 Testing GitClient.merge with an invalid destination branch. ... ok
	+
	+	91 Testing GitClient.merge with an invalid target branch. ... ok
	+
	+	92 Testing GitClient.merge with squash set to True. ... ok
	+
	+	93 Testing GitClient.merge with squash set to False. ... ok
	+
	+	94 Testing GitClient.parse_revision_spec with no specified revisions ... ok
	+
	+	95 Testing GitClient.parse_revision_spec with no specified revisions and a parent diff ... ok
	+
	+	96 Testing GitClient.parse_revision_spec with one specified revision ... ok
	+
	+	97 Testing GitClient.parse_revision_spec with one specified revision and a parent diff ... ok
	+
	+	98 Testing GitClient.parse_revision_spec with diff-since-merge syntax ... ok
	+
	+	99 Testing GitClient.parse_revision_spec with diff-since syntax ... ok
	+
	+	100 Testing GitClient.parse_revision_spec with two specified revisions ... ok
	+
	+	101 Testing GitClient.push_upstream with 'git push' disabled. ... ok
	+
	+	102 Testing GitClient.push_upstream with an invalid remote branch. ... ok
	+
	+	103 Testing GitClient scan_for_server using repo property ... ok
	+
	+	104 Testing GitClient scan_for_server, .reviewboardrc case ... ok
	+
	+	105 Testing GitClient scan_for_server, simple case ... ok
	+
	+	106 Testing GitClient parse_revision_spec with too many revisions ... ok
	+
	+	107 Testing MercurialClient diff with diverged branch ... ok
	+
	+	108 Testing MercurialClient diff with file exclusion ... ok
	+
	+	109 Testing MercurialClient diff with empty file exclusion ... ok
	+
	+	110 Testing MercurialClient parent diffs with a diverged branch ... ok
	+
	+	111 Testing MercurialClient parent diffs with a simple case ... ok
	+
	+	112 Testing MercurialClient parent diffs with a diverged branch and --parent option ... ok
	+
	+	113 Testing MercurialClient diff, simple case ... ok
	+
	+	114 Testing MercurialClient diff with multiple commits ... ok
	+
	+	115 Testing MercurialClient get_repository_info, simple case ... ok
	+
	+	116 Testing MercurialClient guess summary & description 1 commit ... ok
	+
	+	117 Testing MercurialClient guess summary & description middle commit ... ok
	+
	+	118 Testing MercurialClient guess summary & description 3 commits ... ok
	+
	+	119 Testing MercurialClient guess summary & description 2 commits ... ok
	+
	+	120 Testing MercurialClient.parse_revision_spec with no arguments ... ok
	+
	+	121 Testing MercurialClient.parse_revision_spec with one revision ... ok
	+
	+	122 Testing MercurialClient.parse_revision_spec with r1::r2 syntax ... ok
	+
	+	123 Testing MercurialClient.parse_revision_spec with r1..r2 syntax ... ok
	+
	+	124 Testing MercurialClient.parse_revision_spec with parent base ... ok
	+
	+	125 Testing MercurialClient.parse_revision_spec with two revisions ... ok
	+
	+	126 Testing MercurialClient scan_for_server when in .reviewboardrc ... ok
	+
	+	127 Testing MercurialClient scan_for_server, simple case ... ok
	+
	+	128 Testing MercurialClient scan_for_server when present in hgrc ... ok
	+
	+	129 Testing MercurialClient (+svn) _calculate_hgsubversion_repository_info properly determines repository and base paths ... ok
	+
	+	130 Testing MercurialClient (+svn) diff specifying a revision ... ok
	+
	+	131 Testing MercurialClient (+svn) diff, simple case ... ok
	+
	+	132 Testing MercurialClient (+svn) diff with multiple commits ... ok
	+
	+	133 Testing MercurialClient (+svn) get_repository_info, simple case ... ok
	+
	+	134 Testing MercurialClient (+svn) scan_for_server in svn property ... ok
	+
	+	135 Testing MercurialClient (+svn) scan_for_server in .reviewboardrc ... ok
	+
	+	136 Testing MercurialClient (+svn) scan_for_server, simple case ... ok
	+
	+	137 Testing P4Wrapper.counters ... ok
	+
	+	138 Testing P4Wrapper.info ... ok
	+
	+	139 Testing PerforceClient.normalize_exclude_patterns ... ok
	+
	+	140 Testing PerforceClient.diff with a submitted changelist ... ok
	+
	+	141 Testing PerforceClient.diff with moved files and capability off ... ok
	+
	+	142 Testing PerforceClient.diff with moved files and capability on ... ok
	+
	+	143 Testing PerforceClient.diff with a pending changelist ... ok
	+
	+	144 Testing PerforceClient.parse_revision_spec with invalid specifications ... ok
	+
	+	145 Testing PerforceClient.parse_revision_spec with no specified revisions ... ok
	+
	+	146 Testing PerforceClient.parse_revision_spec with a pending changelist ... ok
	+
	+	147 Testing PerforceClient.parse_revision_spec with a shelved changelist ... ok
	+
	+	148 Testing PerforceClient.parse_revision_spec with a submitted changelist ... ok
	+
	+	149 Testing PerforceClient.parse_revision_spec with two changelists ... ok
	+
	+	150 Testing PerforceClient.get_repository_info ... ok
	+
	+	151 Testing PerforceClient.get_repository_info outside client root ... ok
	+
	+	152 Testing PerforceClient.scan_for_server_counter with reviewboard.url ... ok
	+
	+	153 Testing PerforceClient.scan_for_server_counter with encoded reviewboard.url.http:\|\| ... ok
	+
	+	154 Testing SVNClient diff with file exclude patterns ... ok
	+
	+	155 Testing SVNClient diff with exclude patterns in a subdir ... ok
	+
	+	156 Testing SVNClient diff with repo exclude patterns in a subdir ... ok
	+
	+	157 Testing SVNClient diff with a non-utf8 file ... ok
	+
	+	158 Testing SVNClient diff with a non-utf8 filename via repository_url option ... ok
	+
	+	159 Testing SVNClient.get_commit_message with a single committed revision ... ok
	+
	+	160 Testing SVNClient.get_commit_message with multiple committed revisions ... ok
	+
	+	161 Testing SVNClient.get_commit_message with a working copy change ... ok
	+
	+	162 Testing SVNClient.history_scheduled_with_commit nominal cases ... ok
	+
	+	163 Testing SVNClient.history_scheduled_with_commit with exclude file ... ok
	+
	+	164 Testing SVNClient.history_scheduled_with_commit is bypassed when diff is not for local modifications in a working copy ... ok
	+
	+	165 Testing SVNClient.parse_revision_spec with invalid specifications ... ok
	+
	+	166 Testing SVNClient.parse_revision_spec with no specified revisions ... ok
	+
	+	167 Testing SVNClient.parse_revision_spec with a non-utf8 log entry ... ok
	+
	+	168 Testing SVNClient.parse_revision_spec with R1:R2 syntax ... ok
	+
	+	169 Testing SVNClient.parse_revision_spec with one specified numeric revision ... ok
	+
	+	170 Testing SVNClient.parse_revision_spec with one specified changelist revision ... ok
	+
	+	171 Testing SVNClient.parse_revision_spec with one specified invalid changelist revision ... ok
	+
	+	172 Testing SVNClient.parse_revision_spec with one revision and a repository URL ... ok
	+
	+	173 Testing SVNClient.parse_revision_spec with two revisions ... ok
	+
	+	174 Testing SVNClient.parse_revision_spec with R1:R2 syntax and a repository URL ... ok
	+
	+	175 Test diff with removal of lines that look like headers ... ok
	+
	+	176 Testing SVNClient identical diff generated from root, subdirectory, and via target ... ok
	+
	+	177 Testing SVNClient with --show-copies-as-adds functionality disabled ... ok
	+
	+	178 Testing SVNClient with --show-copies-as-adds functionality enabled ... ok
	+
	+	179 Testing SVNRepositoryInfo.find_server_repository_info with mirror_path matching ... ok
	+
	+	180 Testing SVNRepositoryInfo.find_server_repository_info with path matching ... ok
	+
	+	181 Testing SVNRepositoryInfo.find_server_repository_info with UUID matching ... ok
	+
	+	182 Testing SVNRepositoryInfo._get_relative_path ... ok
	+
	+	183 Testing argument duplication with rbt post --field myField=test --description test ... ok
	+
	+	184 Testing multiple delimiters with rbt post --field myField=this=string=has=equals=signs ... ok
	+
	+	185 Testing built-in fields through extra_fields with rbt post --field description=testing --field summary='native testing' --field testing-done='No tests' ... ok
	+
	+	186 Testing setting target group via config file with rbt post ... ok
	+
	+	187 Testing setting target person via config file with rbt post ... ok
	+
	+	188 Testing multiple extra field arguments with rbt post --field foo=bar --field desc=new ... ok
	+
	+	189 Testing setting target group on update via config file with rbt post --review-request-id=12345 ... ok
	+
	+	190 Testing setting target person on update via config file with rbt post --review-request-id=12345 ... ok
	+
	+	191 Testing one extra field argument with rbt post --field foo=bar ... ok
	+
	+	192 Testing setting the target group with rbt post --target-groups=test_group ... ok
	+
	+	193 Testing setting the target group on an update with rbt post --target-groups=test_group --review-request-id=12345 ... ok
	+
	+	194 Testing setting the target person with rbt post --target-people=test_person ... ok
	+
	+	195 Testing setting the target person on an update with rbt post --target-people=test_person --review-request-id=12345 ... ok
	+
	+	196 Testing built-in fields through extra_fields with rbt post --field description and rbt post --field testing_done='No tests' ... ok
	+
	+	197 Testing variable substitution with alphabetic variables ... ok
	+
	+	198 Testing alias substitution with a slash at the end of the string ... ok
	+
	+	199 Testing variable substitution where no variables are supplied ... ok
	+
	+	200 Testing alias substitution with bad quotes. ... ok
	+
	+	201 Testing variable substitution in rbtools aliases ... ok
	+
	+	202 Testing variable substitution where the argument isn't supplied ... ok
	+
	+	203 Testing variable substitution with the $* variable ... ok
	+
	+	204 Testing $* variable substitution with whitespace-containing args ... ok
	+
	+	205 Testing variable substitution where multiple variables appear ... ok
	+
	+	206 Testing 'check_install' method. ... ok
	+
	+	207 Testing 'execute' method. ... ok
	+
	+	208 Testing 'is_valid_version' method. ... ok
	+
	+	209 Testing 'make_empty_files' method. ... ok
	+
	+	210 Testing 'make_tempfile' method. ... ok
	+
	+
	+
	+	Ran 210 tests in 215.007s
	+
	+	OK
	+	./tests/runtests.py 93.50s user 60.71s system 71% cpu 3:35.47 total

Depends On:

-	~~9648 - Fix exception handling, and update imports~~
-	~~9649 - Make six use consistant.~~
-	~~9650 - Fix log messages~~
-	~~9651 - Make quote use consistent across repo~~
-	~~9652 - Fix various things for python 3 compatibility~~
-	~~9653 - Refactor encode_multipart_formdata and tests to be easier to read and maintain~~

Commit:

-	a5ab6d3cb14ddc9007f96ea8c43008db9fc4393b
+	94b1c1ce7355620cf101e17d9cb74dc03a7e5618

Diff:

Revision 5 (+92 -74)

Show changes

	.gitignore
	setup.cfg
	contrib/internal/release.py
	rbtools/api/decode.py
	rbtools/api/request.py
	rbtools/api/tests.py
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	12 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Testing Done:

		(%) flake8 -v tests rbtools

		flake8.checker MainProcess 409 INFO Making checkers
		flake8.checker MainProcess 650 INFO Checking 79 files
		flake8.main.application MainProcess 2483 INFO Finished running
		flake8.main.application MainProcess 2484 INFO Reporting errors
~		flake8.main.application MainProcess 2485 INFO Found a total of 37 violations and reported 0
~		ime ./tests/runtests.py [git:f7e978d...??]??
	~	flake8.main.application MainProcess 2485 INFO Found a total of 37 violations and reported 0
	~
	+	Note: These violations were in appdirs.py and the docs directory.
	+
	+	time ./tests/runtests.py
		/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/pyversion.py:36: DeprecationWarning: The 'new' module has been removed in Python 3.0; use the 'types' module instead.
		import new
		/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/plugins/prof.py:14: DeprecationWarning: The 'hotshot' module is not supported in 3.x, use the 'profile' module instead.
		import hotshot
		/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/pyversion.py:49: DeprecationWarning: Overriding eq blocks inheritance of hash in 3.x
		class Key(object):

		1 Testing the cache with a high max-age value ... /Volumes/Augie/solarmist/rbtools/rbtools/api/cache.py:479: DeprecationWarning: buffer() not supported in 3.x

		sqlite3.Binary(entry.response_body)))
		ok

		2 Testing the cache with a zero max-age value ... /Volumes/Augie/solarmist/rbtools/rbtools/api/cache.py:493: DeprecationWarning: buffer() not supported in 3.x

		sqlite3.Binary(entry.response_body), entry.url,
		ok

		3 Testing the cache with the must-revalidate control ... ok

		4 Testing the cache with the no-store control ... ok

		5 Testing the cache with the no-cache control ... ok

		6 Testing the cache with the no-cache control and a specified ETag ... ok

		7 Testing the cache with the no-cache control and an updated ETag ... ok

		8 Testing the cache with the no-cache control ... ok

		9 Testing the cache with the no-cache control and an updated Last-Modified header ... ok

		10 Testing the cache with the Expires header in the past ... ok

		11 Testing the cache with the Expires header in the future ... ok

		12 Testing the cache with an Expires header that is overridden ... ok

		13 Testing the cache with the Pragma: no-cache header ... ok

		14 Testing writing to the cache with non-ASCII data ... ok

		15 Testing the cache with the Vary header ... ok

		16 Testing the cache with the Vary header and different requests ... ok

		17 Testing Capabilities.has_capability with supported capability ... ok

		18 Testing Capabilities.has_capability with partial capability path ... ok

		19 Testing Capabilities.has_capability with unknown capability ... ok

		20 Testing the default values ... ok

		21 Testing the multipart form data generation ... ok

		22 Testing the encoding of multipart form data with unicode and binary field data ... ok

		23 Testing constructing a count resource ... ok

		24 Testing constructing an item resource ... ok

		25 Testing constructing a list resource ... ok

		26 Testing constructing without guessing the resource token ... ok

		27 Testing constructing a resource with a specific base class ... ok

		28 Testing guessing the resource's token ... ok

		29 Testing rewriting of extra_data__ parameters to create ... ok

		30 Testing rewriting of exta_data__ parameters to update ... ok

		31 Testing item resource fields ... ok

		32 Testing item resource link generation ... ok

		33 Testing access of a link field ... ok

		34 Testing link resource link generation ... ok

		35 Testing list resource lists ... ok

		36 Testing access of a dictionary field ... ok

		37 Testing iterating sub-fields of a dictionary field ... ok

		38 Testing generation of methods for the root resource uri templates ... ok

		39 Testing ReviewRequestResource.absolute_url with 'absolute_url' field ... ok

		40 Testing ReviewRequestResource.absolute_url with generated fallback URL ... ok

		41 Testing ReviewRequestResource.absolute_url with 'url' field ... ok

		42 Testing BazaarClient diff with file exclusion ... ok

		43 Testing BazaarClient diff with file exclusion in a subdirectory ... ok

		44 Testing BazaarClient diff with file exclusion in the repo root ... ok

		45 Testing BazaarClient diff with changes between a 2nd level descendant ... ok

		46 Testing BazaarClient diff with changes only in the parent branch ... ok

		47 Testing BazaarClient simple diff case ... ok

		48 Testing BazaarClient simple diff with multiple commits case ... ok

		49 Testing BazaarClient diff with specific files ... ok

		50 Testing BazaarClient get_repository_info with child branch ... ok

		51 Testing BazaarClient get_repository_info, no branch ... ok

		52 Testing BazaarClient get_repository_info with original branch ... ok

		53 Testing BazaarClient guessing summary and description ... ok

		54 Testing BazaarClient guessing summary and description for grand parent branch ... ok

		55 Testing BazaarClient guessing summary and description with a revision range ... ok

		56 Testing BazaarClient.parse_revision_spec with no specified revisions ... ok

		57 Testing BazaarClient.parse_revision_spec with one specified revision ... ok

		58 Testing BazaarClient.parse_revision_spec with one specified revision and a parent diff ... ok

		59 Testing BazaarClient.parse_revision_spec with R1..R2 syntax ... ok

		60 Testing BazaarClient.parse_revision_spec with two revisions ... ok

		61 Testing BazaarClient parse_revision_spec with too many revisions ... ok

		62 Testing GitClient.create_commit with all_files set to True. ... ok

		63 Testing GitClient.create_commit with run_editor set to True. ... ok

		64 Testing GitClient.create_commit with all_files set to False. ... ok

		65 Testing GitClient.create_commit with run_editor set to False. ... ok

		66 Testing GitClient.delete_branch with merged_only set to True. ... ok

		67 Testing GitClient.delete_branch with merged_only set to False. ... ok

		68 Testing GitClient diff with divergent branches ... ok

		69 Testing GitClient simple diff with file exclusion ... ok

		70 Testing GitClient simple diff with file exclusion in a subdir ... ok

		71 Testing GitClient diff with file exclusion in the repo root ... ok

		72 Testing GitClient.parse_revision_spec with target branch off a tracking branch not aligned with the remote ... ok

		73 Testing GitClient.parse_revision_spec with a target branch not up-to-date with a remote branch ... ok

		74 Testing GitClient.parse_revision_spec with a target branch posted off a tracking branch that merged another tracking branch ... ok

		75 Testing GitClient.parse_revision_spec with a target branch that merged a tracking branch off another tracking branch ... ok

		76 Testing GitClient.parse_revision_spec with a target branch that has branches from different remotes in its path ... ok

		77 Testing GitClient.parse_revision_spec with target branch off a tracking branch aligned with the remote ... ok

		78 Testing GitClient.parse_revision_spec with a target branch posted off a remote branch that is aligned to the same commit as another remote branch ... ok

		79 Testing GitClient.parse_revision_spec with a target branch posted off a remote branch without any tracking branches ... ok

		80 Testing GitClient.parse_revision_spec with target branch off a branch not properly tracking the remote ... ok

		81 Testing GitClient.parse_revision_spec with target branch off a tracking branch with changes since the remote ... ok

		82 Testing GitClient diff with a local tracking branch ... ok

		83 Testing GitClient simple diff case ... ok

		84 Testing GitClient simple diff with multiple commits case ... ok

		85 Testing GitClient diff with tracking branch that has slash in its name ... ok

		86 Testing GitClient diff with a tracking branch, but no origin remote ... ok

		87 Testing GitClient diff with option override for tracking branch ... ok

		88 Testing GitClient.get_raw_commit_message ... ok

		89 Testing GitClient get_repository_info, simple case ... ok

		90 Testing GitClient.merge with an invalid destination branch. ... ok

		91 Testing GitClient.merge with an invalid target branch. ... ok

		92 Testing GitClient.merge with squash set to True. ... ok

		93 Testing GitClient.merge with squash set to False. ... ok

		94 Testing GitClient.parse_revision_spec with no specified revisions ... ok

		95 Testing GitClient.parse_revision_spec with no specified revisions and a parent diff ... ok

		96 Testing GitClient.parse_revision_spec with one specified revision ... ok

		97 Testing GitClient.parse_revision_spec with one specified revision and a parent diff ... ok

		98 Testing GitClient.parse_revision_spec with diff-since-merge syntax ... ok

		99 Testing GitClient.parse_revision_spec with diff-since syntax ... ok

		100 Testing GitClient.parse_revision_spec with two specified revisions ... ok

		101 Testing GitClient.push_upstream with 'git push' disabled. ... ok

		102 Testing GitClient.push_upstream with an invalid remote branch. ... ok

		103 Testing GitClient scan_for_server using repo property ... ok

		104 Testing GitClient scan_for_server, .reviewboardrc case ... ok

		105 Testing GitClient scan_for_server, simple case ... ok

		106 Testing GitClient parse_revision_spec with too many revisions ... ok

		107 Testing MercurialClient diff with diverged branch ... ok

		108 Testing MercurialClient diff with file exclusion ... ok

		109 Testing MercurialClient diff with empty file exclusion ... ok

		110 Testing MercurialClient parent diffs with a diverged branch ... ok

		111 Testing MercurialClient parent diffs with a simple case ... ok

		112 Testing MercurialClient parent diffs with a diverged branch and --parent option ... ok

		113 Testing MercurialClient diff, simple case ... ok

		114 Testing MercurialClient diff with multiple commits ... ok

		115 Testing MercurialClient get_repository_info, simple case ... ok

		116 Testing MercurialClient guess summary & description 1 commit ... ok

		117 Testing MercurialClient guess summary & description middle commit ... ok

		118 Testing MercurialClient guess summary & description 3 commits ... ok

		119 Testing MercurialClient guess summary & description 2 commits ... ok

		120 Testing MercurialClient.parse_revision_spec with no arguments ... ok

		121 Testing MercurialClient.parse_revision_spec with one revision ... ok

		122 Testing MercurialClient.parse_revision_spec with r1::r2 syntax ... ok

		123 Testing MercurialClient.parse_revision_spec with r1..r2 syntax ... ok

		124 Testing MercurialClient.parse_revision_spec with parent base ... ok

		125 Testing MercurialClient.parse_revision_spec with two revisions ... ok

		126 Testing MercurialClient scan_for_server when in .reviewboardrc ... ok

		127 Testing MercurialClient scan_for_server, simple case ... ok

		128 Testing MercurialClient scan_for_server when present in hgrc ... ok

		129 Testing MercurialClient (+svn) _calculate_hgsubversion_repository_info properly determines repository and base paths ... ok

		130 Testing MercurialClient (+svn) diff specifying a revision ... ok

		131 Testing MercurialClient (+svn) diff, simple case ... ok

		132 Testing MercurialClient (+svn) diff with multiple commits ... ok

		133 Testing MercurialClient (+svn) get_repository_info, simple case ... ok

		134 Testing MercurialClient (+svn) scan_for_server in svn property ... ok

		135 Testing MercurialClient (+svn) scan_for_server in .reviewboardrc ... ok

		136 Testing MercurialClient (+svn) scan_for_server, simple case ... ok

		137 Testing P4Wrapper.counters ... ok

		138 Testing P4Wrapper.info ... ok

		139 Testing PerforceClient.normalize_exclude_patterns ... ok

		140 Testing PerforceClient.diff with a submitted changelist ... ok

		141 Testing PerforceClient.diff with moved files and capability off ... ok

		142 Testing PerforceClient.diff with moved files and capability on ... ok

		143 Testing PerforceClient.diff with a pending changelist ... ok

		144 Testing PerforceClient.parse_revision_spec with invalid specifications ... ok

		145 Testing PerforceClient.parse_revision_spec with no specified revisions ... ok

		146 Testing PerforceClient.parse_revision_spec with a pending changelist ... ok

		147 Testing PerforceClient.parse_revision_spec with a shelved changelist ... ok

		148 Testing PerforceClient.parse_revision_spec with a submitted changelist ... ok

		149 Testing PerforceClient.parse_revision_spec with two changelists ... ok

		150 Testing PerforceClient.get_repository_info ... ok

		151 Testing PerforceClient.get_repository_info outside client root ... ok

		152 Testing PerforceClient.scan_for_server_counter with reviewboard.url ... ok

		153 Testing PerforceClient.scan_for_server_counter with encoded reviewboard.url.http:\|\| ... ok

		154 Testing SVNClient diff with file exclude patterns ... ok

		155 Testing SVNClient diff with exclude patterns in a subdir ... ok

		156 Testing SVNClient diff with repo exclude patterns in a subdir ... ok

		157 Testing SVNClient diff with a non-utf8 file ... ok

		158 Testing SVNClient diff with a non-utf8 filename via repository_url option ... ok

		159 Testing SVNClient.get_commit_message with a single committed revision ... ok

		160 Testing SVNClient.get_commit_message with multiple committed revisions ... ok

		161 Testing SVNClient.get_commit_message with a working copy change ... ok

		162 Testing SVNClient.history_scheduled_with_commit nominal cases ... ok

		163 Testing SVNClient.history_scheduled_with_commit with exclude file ... ok

		164 Testing SVNClient.history_scheduled_with_commit is bypassed when diff is not for local modifications in a working copy ... ok

		165 Testing SVNClient.parse_revision_spec with invalid specifications ... ok

		166 Testing SVNClient.parse_revision_spec with no specified revisions ... ok

		167 Testing SVNClient.parse_revision_spec with a non-utf8 log entry ... ok

		168 Testing SVNClient.parse_revision_spec with R1:R2 syntax ... ok

		169 Testing SVNClient.parse_revision_spec with one specified numeric revision ... ok

		170 Testing SVNClient.parse_revision_spec with one specified changelist revision ... ok

		171 Testing SVNClient.parse_revision_spec with one specified invalid changelist revision ... ok

		172 Testing SVNClient.parse_revision_spec with one revision and a repository URL ... ok

		173 Testing SVNClient.parse_revision_spec with two revisions ... ok

		174 Testing SVNClient.parse_revision_spec with R1:R2 syntax and a repository URL ... ok

		175 Test diff with removal of lines that look like headers ... ok

		176 Testing SVNClient identical diff generated from root, subdirectory, and via target ... ok

		177 Testing SVNClient with --show-copies-as-adds functionality disabled ... ok

		178 Testing SVNClient with --show-copies-as-adds functionality enabled ... ok

		179 Testing SVNRepositoryInfo.find_server_repository_info with mirror_path matching ... ok

		180 Testing SVNRepositoryInfo.find_server_repository_info with path matching ... ok

		181 Testing SVNRepositoryInfo.find_server_repository_info with UUID matching ... ok

		182 Testing SVNRepositoryInfo._get_relative_path ... ok

		183 Testing argument duplication with rbt post --field myField=test --description test ... ok

		184 Testing multiple delimiters with rbt post --field myField=this=string=has=equals=signs ... ok

		185 Testing built-in fields through extra_fields with rbt post --field description=testing --field summary='native testing' --field testing-done='No tests' ... ok

		186 Testing setting target group via config file with rbt post ... ok

		187 Testing setting target person via config file with rbt post ... ok

		188 Testing multiple extra field arguments with rbt post --field foo=bar --field desc=new ... ok

		189 Testing setting target group on update via config file with rbt post --review-request-id=12345 ... ok

		190 Testing setting target person on update via config file with rbt post --review-request-id=12345 ... ok

		191 Testing one extra field argument with rbt post --field foo=bar ... ok

		192 Testing setting the target group with rbt post --target-groups=test_group ... ok

		193 Testing setting the target group on an update with rbt post --target-groups=test_group --review-request-id=12345 ... ok

		194 Testing setting the target person with rbt post --target-people=test_person ... ok

		195 Testing setting the target person on an update with rbt post --target-people=test_person --review-request-id=12345 ... ok

		196 Testing built-in fields through extra_fields with rbt post --field description and rbt post --field testing_done='No tests' ... ok

		197 Testing variable substitution with alphabetic variables ... ok

		198 Testing alias substitution with a slash at the end of the string ... ok

		199 Testing variable substitution where no variables are supplied ... ok

		200 Testing alias substitution with bad quotes. ... ok

		201 Testing variable substitution in rbtools aliases ... ok

		202 Testing variable substitution where the argument isn't supplied ... ok

		203 Testing variable substitution with the $* variable ... ok

		204 Testing $* variable substitution with whitespace-containing args ... ok

		205 Testing variable substitution where multiple variables appear ... ok

		206 Testing 'check_install' method. ... ok

		207 Testing 'execute' method. ... ok

		208 Testing 'is_valid_version' method. ... ok

		209 Testing 'make_empty_files' method. ... ok

		210 Testing 'make_tempfile' method. ... ok



		Ran 210 tests in 215.007s

		OK
		./tests/runtests.py 93.50s user 60.71s system 71% cpu 3:35.47 total

The issue has been resolved. Show all issues

You really don't need the entire output of the tests in the testing done. You can just say you ran the tests and they passed.

solarmist Feb. 22, 2018, 3:44 p.m.

Done. I just left tests that had any output. Most of it is just python 3 warnings.

.gitignore (Diff revision 5)
The issue has been resolved. Show all issues
```
This is a wildcard and so should be under the list of wildcards.
```
1. solarmist Feb. 22, 2018, 3:44 p.m.
  Will do.
2. solarmist March 2, 2018, 10:52 p.m.
  Sorted.

rbtools/api/request.py (Diff revision 5)

The issue has been dropped. Show all issues

Instead, how about:

content.write(b'%s%s'
              % (six.text_type(self._fields[key]).encode('utf-8'),
              NEWLINE)

solarmist Feb. 22, 2018, 3:44 p.m.

I have a seperate review to refactor this code, so I'd like to only do the formatting update in this change.

rbtools/api/request.py (Diff revision 5)

The issue has been dropped. Show all issues

Instead, how about:

content.write(b'filename="%s"%s'
              % (filename.encode('utf-8'), NEWLINE))

solarmist Feb. 22, 2018, 3:44 p.m.
```
Ditto
```

Testing Done:

		(%) flake8 -v tests rbtools

		flake8.checker MainProcess 409 INFO Making checkers
		flake8.checker MainProcess 650 INFO Checking 79 files
		flake8.main.application MainProcess 2483 INFO Finished running
		flake8.main.application MainProcess 2484 INFO Reporting errors
		flake8.main.application MainProcess 2485 INFO Found a total of 37 violations and reported 0

		Note: These violations were in appdirs.py and the docs directory.

		time ./tests/runtests.py
		/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/pyversion.py:36: DeprecationWarning: The 'new' module has been removed in Python 3.0; use the 'types' module instead.
		import new
		/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/plugins/prof.py:14: DeprecationWarning: The 'hotshot' module is not supported in 3.x, use the 'profile' module instead.
		import hotshot
~		/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/pyversion.py:49: DeprecationWarning: Overriding eq blocks inheritance of hash in 3.x
~		class Key(object):
~
~		1 Testing the cache with a high max-age value ... /Volumes/Augie/solarmist/rbtools/rbtools/api/cache.py:479: DeprecationWarning: buffer() not supported in 3.x
~
~		sqlite3.Binary(entry.response_body)))
	~	/Volumes/Augie/solarmist/rbtools/lib/python2.7/site-packages/nose/pyversion.py:49: DeprecationWarning: Overriding __eq__ blocks inheritance of __hash__ in 3.x
	~	class Key(object):
	~	#1 Testing the cache with a high max-age value ... /Volumes/Augie/solarmist/rbtools/rbtools/api/cache.py:479: DeprecationWarning: buffer() not supported in 3.x
	~	sqlite3.Binary(entry.response_body)))
	~	ok
	~	#2 Testing the cache with a zero max-age value ... /Volumes/Augie/solarmist/rbtools/rbtools/api/cache.py:493: DeprecationWarning: buffer() not supported in 3.x
	+	sqlite3.Binary(entry.response_body), entry.url,
		ok

-		2 Testing the cache with a zero max-age value ... /Volumes/Augie/solarmist/rbtools/rbtools/api/cache.py:493: DeprecationWarning: buffer() not supported in 3.x
-
-		sqlite3.Binary(entry.response_body), entry.url,
-		ok
-
-		3 Testing the cache with the must-revalidate control ... ok
-
-		4 Testing the cache with the no-store control ... ok
-
-		5 Testing the cache with the no-cache control ... ok
-
-		6 Testing the cache with the no-cache control and a specified ETag ... ok
-
-		7 Testing the cache with the no-cache control and an updated ETag ... ok
-
-		8 Testing the cache with the no-cache control ... ok
-
-		9 Testing the cache with the no-cache control and an updated Last-Modified header ... ok
-
-		10 Testing the cache with the Expires header in the past ... ok
-
-		11 Testing the cache with the Expires header in the future ... ok
-
-		12 Testing the cache with an Expires header that is overridden ... ok
-
-		13 Testing the cache with the Pragma: no-cache header ... ok
-
-		14 Testing writing to the cache with non-ASCII data ... ok
-
-		15 Testing the cache with the Vary header ... ok
-
-		16 Testing the cache with the Vary header and different requests ... ok
-
-		17 Testing Capabilities.has_capability with supported capability ... ok
-
-		18 Testing Capabilities.has_capability with partial capability path ... ok
-
-		19 Testing Capabilities.has_capability with unknown capability ... ok
-
-		20 Testing the default values ... ok
-
-		21 Testing the multipart form data generation ... ok
-
-		22 Testing the encoding of multipart form data with unicode and binary field data ... ok
-
-		23 Testing constructing a count resource ... ok
-
-		24 Testing constructing an item resource ... ok
-
-		25 Testing constructing a list resource ... ok
-
-		26 Testing constructing without guessing the resource token ... ok
-
-		27 Testing constructing a resource with a specific base class ... ok
-
-		28 Testing guessing the resource's token ... ok
-
-		29 Testing rewriting of extra_data__ parameters to create ... ok
-
-		30 Testing rewriting of exta_data__ parameters to update ... ok
-
-		31 Testing item resource fields ... ok
-
-		32 Testing item resource link generation ... ok
-
-		33 Testing access of a link field ... ok
-
-		34 Testing link resource link generation ... ok
-
-		35 Testing list resource lists ... ok
-
-		36 Testing access of a dictionary field ... ok
-
-		37 Testing iterating sub-fields of a dictionary field ... ok
-
-		38 Testing generation of methods for the root resource uri templates ... ok
-
-		39 Testing ReviewRequestResource.absolute_url with 'absolute_url' field ... ok
-
-		40 Testing ReviewRequestResource.absolute_url with generated fallback URL ... ok
-
-		41 Testing ReviewRequestResource.absolute_url with 'url' field ... ok
-
-		42 Testing BazaarClient diff with file exclusion ... ok
-
-		43 Testing BazaarClient diff with file exclusion in a subdirectory ... ok
-
-		44 Testing BazaarClient diff with file exclusion in the repo root ... ok
-
-		45 Testing BazaarClient diff with changes between a 2nd level descendant ... ok
-
-		46 Testing BazaarClient diff with changes only in the parent branch ... ok
-
-		47 Testing BazaarClient simple diff case ... ok
-
-		48 Testing BazaarClient simple diff with multiple commits case ... ok
-
-		49 Testing BazaarClient diff with specific files ... ok
-
-		50 Testing BazaarClient get_repository_info with child branch ... ok
-
-		51 Testing BazaarClient get_repository_info, no branch ... ok
-
-		52 Testing BazaarClient get_repository_info with original branch ... ok
-
-		53 Testing BazaarClient guessing summary and description ... ok
-
-		54 Testing BazaarClient guessing summary and description for grand parent branch ... ok
-
-		55 Testing BazaarClient guessing summary and description with a revision range ... ok
-
-		56 Testing BazaarClient.parse_revision_spec with no specified revisions ... ok
-
-		57 Testing BazaarClient.parse_revision_spec with one specified revision ... ok
-
-		58 Testing BazaarClient.parse_revision_spec with one specified revision and a parent diff ... ok
-
-		59 Testing BazaarClient.parse_revision_spec with R1..R2 syntax ... ok
-
-		60 Testing BazaarClient.parse_revision_spec with two revisions ... ok
-
-		61 Testing BazaarClient parse_revision_spec with too many revisions ... ok
-
-		62 Testing GitClient.create_commit with all_files set to True. ... ok
-
-		63 Testing GitClient.create_commit with run_editor set to True. ... ok
-
-		64 Testing GitClient.create_commit with all_files set to False. ... ok
-
-		65 Testing GitClient.create_commit with run_editor set to False. ... ok
-
-		66 Testing GitClient.delete_branch with merged_only set to True. ... ok
-
-		67 Testing GitClient.delete_branch with merged_only set to False. ... ok
-
-		68 Testing GitClient diff with divergent branches ... ok
-
-		69 Testing GitClient simple diff with file exclusion ... ok
-
-		70 Testing GitClient simple diff with file exclusion in a subdir ... ok
-
-		71 Testing GitClient diff with file exclusion in the repo root ... ok
-
-		72 Testing GitClient.parse_revision_spec with target branch off a tracking branch not aligned with the remote ... ok
-
-		73 Testing GitClient.parse_revision_spec with a target branch not up-to-date with a remote branch ... ok
-
-		74 Testing GitClient.parse_revision_spec with a target branch posted off a tracking branch that merged another tracking branch ... ok
-
-		75 Testing GitClient.parse_revision_spec with a target branch that merged a tracking branch off another tracking branch ... ok
-
-		76 Testing GitClient.parse_revision_spec with a target branch that has branches from different remotes in its path ... ok
-
-		77 Testing GitClient.parse_revision_spec with target branch off a tracking branch aligned with the remote ... ok
-
-		78 Testing GitClient.parse_revision_spec with a target branch posted off a remote branch that is aligned to the same commit as another remote branch ... ok
-
-		79 Testing GitClient.parse_revision_spec with a target branch posted off a remote branch without any tracking branches ... ok
-
-		80 Testing GitClient.parse_revision_spec with target branch off a branch not properly tracking the remote ... ok
-
-		81 Testing GitClient.parse_revision_spec with target branch off a tracking branch with changes since the remote ... ok
-
-		82 Testing GitClient diff with a local tracking branch ... ok
-
-		83 Testing GitClient simple diff case ... ok
-
-		84 Testing GitClient simple diff with multiple commits case ... ok
-
-		85 Testing GitClient diff with tracking branch that has slash in its name ... ok
-
-		86 Testing GitClient diff with a tracking branch, but no origin remote ... ok
-
-		87 Testing GitClient diff with option override for tracking branch ... ok
-
-		88 Testing GitClient.get_raw_commit_message ... ok
-
-		89 Testing GitClient get_repository_info, simple case ... ok
-
-		90 Testing GitClient.merge with an invalid destination branch. ... ok
-
-		91 Testing GitClient.merge with an invalid target branch. ... ok
-
-		92 Testing GitClient.merge with squash set to True. ... ok
-
-		93 Testing GitClient.merge with squash set to False. ... ok
-
-		94 Testing GitClient.parse_revision_spec with no specified revisions ... ok
-
-		95 Testing GitClient.parse_revision_spec with no specified revisions and a parent diff ... ok
-
-		96 Testing GitClient.parse_revision_spec with one specified revision ... ok
-
-		97 Testing GitClient.parse_revision_spec with one specified revision and a parent diff ... ok
-
-		98 Testing GitClient.parse_revision_spec with diff-since-merge syntax ... ok
-
-		99 Testing GitClient.parse_revision_spec with diff-since syntax ... ok
-
-		100 Testing GitClient.parse_revision_spec with two specified revisions ... ok
-
-		101 Testing GitClient.push_upstream with 'git push' disabled. ... ok
-
-		102 Testing GitClient.push_upstream with an invalid remote branch. ... ok
-
-		103 Testing GitClient scan_for_server using repo property ... ok
-
-		104 Testing GitClient scan_for_server, .reviewboardrc case ... ok
-
-		105 Testing GitClient scan_for_server, simple case ... ok
-
-		106 Testing GitClient parse_revision_spec with too many revisions ... ok
-
-		107 Testing MercurialClient diff with diverged branch ... ok
-
-		108 Testing MercurialClient diff with file exclusion ... ok
-
-		109 Testing MercurialClient diff with empty file exclusion ... ok
-
-		110 Testing MercurialClient parent diffs with a diverged branch ... ok
-
-		111 Testing MercurialClient parent diffs with a simple case ... ok
-
-		112 Testing MercurialClient parent diffs with a diverged branch and --parent option ... ok
-
-		113 Testing MercurialClient diff, simple case ... ok
-
-		114 Testing MercurialClient diff with multiple commits ... ok
-
-		115 Testing MercurialClient get_repository_info, simple case ... ok
-
-		116 Testing MercurialClient guess summary & description 1 commit ... ok
-
-		117 Testing MercurialClient guess summary & description middle commit ... ok
-
-		118 Testing MercurialClient guess summary & description 3 commits ... ok
-
-		119 Testing MercurialClient guess summary & description 2 commits ... ok
-
-		120 Testing MercurialClient.parse_revision_spec with no arguments ... ok
-
-		121 Testing MercurialClient.parse_revision_spec with one revision ... ok
-
-		122 Testing MercurialClient.parse_revision_spec with r1::r2 syntax ... ok
-
-		123 Testing MercurialClient.parse_revision_spec with r1..r2 syntax ... ok
-
-		124 Testing MercurialClient.parse_revision_spec with parent base ... ok
-
-		125 Testing MercurialClient.parse_revision_spec with two revisions ... ok
-
-		126 Testing MercurialClient scan_for_server when in .reviewboardrc ... ok
-
-		127 Testing MercurialClient scan_for_server, simple case ... ok
-
-		128 Testing MercurialClient scan_for_server when present in hgrc ... ok
-
-		129 Testing MercurialClient (+svn) _calculate_hgsubversion_repository_info properly determines repository and base paths ... ok
-
-		130 Testing MercurialClient (+svn) diff specifying a revision ... ok
-
-		131 Testing MercurialClient (+svn) diff, simple case ... ok
-
-		132 Testing MercurialClient (+svn) diff with multiple commits ... ok
-
-		133 Testing MercurialClient (+svn) get_repository_info, simple case ... ok
-
-		134 Testing MercurialClient (+svn) scan_for_server in svn property ... ok
-
-		135 Testing MercurialClient (+svn) scan_for_server in .reviewboardrc ... ok
-
-		136 Testing MercurialClient (+svn) scan_for_server, simple case ... ok
-
-		137 Testing P4Wrapper.counters ... ok
-
-		138 Testing P4Wrapper.info ... ok
-
-		139 Testing PerforceClient.normalize_exclude_patterns ... ok
-
-		140 Testing PerforceClient.diff with a submitted changelist ... ok
-
-		141 Testing PerforceClient.diff with moved files and capability off ... ok
-
-		142 Testing PerforceClient.diff with moved files and capability on ... ok
-
-		143 Testing PerforceClient.diff with a pending changelist ... ok
-
-		144 Testing PerforceClient.parse_revision_spec with invalid specifications ... ok
-
-		145 Testing PerforceClient.parse_revision_spec with no specified revisions ... ok
-
-		146 Testing PerforceClient.parse_revision_spec with a pending changelist ... ok
-
-		147 Testing PerforceClient.parse_revision_spec with a shelved changelist ... ok
-
-		148 Testing PerforceClient.parse_revision_spec with a submitted changelist ... ok
-
-		149 Testing PerforceClient.parse_revision_spec with two changelists ... ok
-
-		150 Testing PerforceClient.get_repository_info ... ok
-
-		151 Testing PerforceClient.get_repository_info outside client root ... ok
-
-		152 Testing PerforceClient.scan_for_server_counter with reviewboard.url ... ok
-
-		153 Testing PerforceClient.scan_for_server_counter with encoded reviewboard.url.http:\|\| ... ok
-
-		154 Testing SVNClient diff with file exclude patterns ... ok
-
-		155 Testing SVNClient diff with exclude patterns in a subdir ... ok
-
-		156 Testing SVNClient diff with repo exclude patterns in a subdir ... ok
-
-		157 Testing SVNClient diff with a non-utf8 file ... ok
-
-		158 Testing SVNClient diff with a non-utf8 filename via repository_url option ... ok
-
-		159 Testing SVNClient.get_commit_message with a single committed revision ... ok
-
-		160 Testing SVNClient.get_commit_message with multiple committed revisions ... ok
-
-		161 Testing SVNClient.get_commit_message with a working copy change ... ok
-
-		162 Testing SVNClient.history_scheduled_with_commit nominal cases ... ok
-
-		163 Testing SVNClient.history_scheduled_with_commit with exclude file ... ok
-
-		164 Testing SVNClient.history_scheduled_with_commit is bypassed when diff is not for local modifications in a working copy ... ok
-
-		165 Testing SVNClient.parse_revision_spec with invalid specifications ... ok
-
-		166 Testing SVNClient.parse_revision_spec with no specified revisions ... ok
-
-		167 Testing SVNClient.parse_revision_spec with a non-utf8 log entry ... ok
-
-		168 Testing SVNClient.parse_revision_spec with R1:R2 syntax ... ok
-
-		169 Testing SVNClient.parse_revision_spec with one specified numeric revision ... ok
-
-		170 Testing SVNClient.parse_revision_spec with one specified changelist revision ... ok
-
-		171 Testing SVNClient.parse_revision_spec with one specified invalid changelist revision ... ok
-
-		172 Testing SVNClient.parse_revision_spec with one revision and a repository URL ... ok
-
-		173 Testing SVNClient.parse_revision_spec with two revisions ... ok
-
-		174 Testing SVNClient.parse_revision_spec with R1:R2 syntax and a repository URL ... ok
-
-		175 Test diff with removal of lines that look like headers ... ok
-
-		176 Testing SVNClient identical diff generated from root, subdirectory, and via target ... ok
-
-		177 Testing SVNClient with --show-copies-as-adds functionality disabled ... ok
-
-		178 Testing SVNClient with --show-copies-as-adds functionality enabled ... ok
-
-		179 Testing SVNRepositoryInfo.find_server_repository_info with mirror_path matching ... ok
-
-		180 Testing SVNRepositoryInfo.find_server_repository_info with path matching ... ok
-
-		181 Testing SVNRepositoryInfo.find_server_repository_info with UUID matching ... ok
-
-		182 Testing SVNRepositoryInfo._get_relative_path ... ok
-
-		183 Testing argument duplication with rbt post --field myField=test --description test ... ok
-
-		184 Testing multiple delimiters with rbt post --field myField=this=string=has=equals=signs ... ok
-
-		185 Testing built-in fields through extra_fields with rbt post --field description=testing --field summary='native testing' --field testing-done='No tests' ... ok
-
-		186 Testing setting target group via config file with rbt post ... ok
-
-		187 Testing setting target person via config file with rbt post ... ok
-
-		188 Testing multiple extra field arguments with rbt post --field foo=bar --field desc=new ... ok
-
-		189 Testing setting target group on update via config file with rbt post --review-request-id=12345 ... ok
-
-		190 Testing setting target person on update via config file with rbt post --review-request-id=12345 ... ok
-
-		191 Testing one extra field argument with rbt post --field foo=bar ... ok
-
-		192 Testing setting the target group with rbt post --target-groups=test_group ... ok
-
-		193 Testing setting the target group on an update with rbt post --target-groups=test_group --review-request-id=12345 ... ok
-
-		194 Testing setting the target person with rbt post --target-people=test_person ... ok
-
-		195 Testing setting the target person on an update with rbt post --target-people=test_person --review-request-id=12345 ... ok
-
-		196 Testing built-in fields through extra_fields with rbt post --field description and rbt post --field testing_done='No tests' ... ok
-
-		197 Testing variable substitution with alphabetic variables ... ok
-
-		198 Testing alias substitution with a slash at the end of the string ... ok
-
-		199 Testing variable substitution where no variables are supplied ... ok
-
-		200 Testing alias substitution with bad quotes. ... ok
-
-		201 Testing variable substitution in rbtools aliases ... ok
-
-		202 Testing variable substitution where the argument isn't supplied ... ok
-
-		203 Testing variable substitution with the $* variable ... ok
-
-		204 Testing $* variable substitution with whitespace-containing args ... ok
-
-		205 Testing variable substitution where multiple variables appear ... ok
-
-		206 Testing 'check_install' method. ... ok
-
-		207 Testing 'execute' method. ... ok
-
-		208 Testing 'is_valid_version' method. ... ok
-
-		209 Testing 'make_empty_files' method. ... ok
-
-		210 Testing 'make_tempfile' method. ... ok
-
-
-
		Ran 210 tests in 215.007s

		OK
		./tests/runtests.py 93.50s user 60.71s system 71% cpu 3:35.47 total

Change Summary:

Sorted .gitignore file

Commit:

-	94b1c1ce7355620cf101e17d9cb74dc03a7e5618
+	820dc84dffb3c79db3d2a512aeb83dff6b7f0258

Diff:

Revision 6 (+98 -80)

Show changes

	.gitignore
	setup.cfg
	contrib/internal/release.py
	rbtools/api/decode.py
	rbtools/api/request.py
	rbtools/api/tests.py
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	12 more

Commit:

-	820dc84dffb3c79db3d2a512aeb83dff6b7f0258
+	8d80f2413049d0aa2e1557f168ce2819d33f459d

Diff:

Revision 7 (+98 -80)

Show changes

	.gitignore
	setup.cfg
	contrib/internal/release.py
	rbtools/api/decode.py
	rbtools/api/request.py
	rbtools/api/tests.py
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	12 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/api/request.py (Diff revision 7)

The issue has been resolved. Show all issues

The way this is wrapped is now more confusing. How about:

content.write(
    six.text_type(self._fields[key]).encode('utf-8') +
    NEWLINE)

setup.cfg (Diff revision 7)

The issue has been resolved. Show all issues

To be consistent with our other configurations, let's do E121,E125,E129,E241

Commit:

-	8d80f2413049d0aa2e1557f168ce2819d33f459d
+	0646f4d49694bc7b6979bbfcc5a64c9632b82a41

Diff:

Revision 8 (+99 -80)

Show changes

	.gitignore
	setup.cfg
	contrib/internal/release.py
	rbtools/api/decode.py
	rbtools/api/request.py
	rbtools/api/tests.py
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	12 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/api/request.py (Diff revision 8)

The issue has been resolved. Show all issues

This still isn't how I suggested you wrap it.

content.write(
    six.text_type(self._fields[key]).encode('utf-8') +
    NEWLINE)

solarmist March 24, 2018, 3:01 p.m.

This is also not a useful comment, let alone a big enough to warrent an issue.  It is a completely non-functional change that doesn't significantly alter the readibility of the statement.  The best I can say is that it's different.

rbtools/clients/git.py (Diff revision 8)

The issue has been resolved. Show all issues

This can be a little bit more compact:

if (git_top.startswith(('fatal:', 'cygdrive')) or
    not os.path.isdir(git_dir)):

solarmist March 24, 2018, 3:01 p.m.

I always forget about this version of the function. It's a bit strange to need to wrap it in a tuple.

rbtools/clients/tests/test_svn.py (Diff revision 8)
The issue has been resolved. Show all issues
```
Extra line here isn't necessary unless there's a class docstring.
```

Commit:

-	0646f4d49694bc7b6979bbfcc5a64c9632b82a41
+	654044192604d823d43f00cfe1bf33508cd8d17f

Diff:

Revision 9 (+97 -80)

Show changes

	.gitignore
	setup.cfg
	contrib/internal/release.py
	rbtools/api/decode.py
	rbtools/api/request.py
	rbtools/api/tests.py
	rbtools/clients/__init__.py
	rbtools/clients/clearcase.py
	12 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to master (0c562dc)

Fix various PEP 8, style and PyFlakes issues

Information

Reviewers

Depends On:

Description:

Testing Done:

Commit:

Diff:

Checks run (2 succeeded)

Depends On:

Summary:

Commit:

Diff:

Checks run (2 succeeded)

Change Summary:

Commit:

Diff:

Checks run (2 succeeded)

Change Summary:

Testing Done:

1 Testing the cache with a high max-age value ... /Volumes/Augie/solarmist/rbtools/rbtools/api/cache.py:479: DeprecationWarning: buffer() not supported in 3.x

2 Testing the cache with a zero max-age value ... /Volumes/Augie/solarmist/rbtools/rbtools/api/cache.py:493: DeprecationWarning: buffer() not supported in 3.x

3 Testing the cache with the must-revalidate control ... ok

4 Testing the cache with the no-store control ... ok

5 Testing the cache with the no-cache control ... ok

6 Testing the cache with the no-cache control and a specified ETag ... ok

7 Testing the cache with the no-cache control and an updated ETag ... ok

8 Testing the cache with the no-cache control ... ok

9 Testing the cache with the no-cache control and an updated Last-Modified header ... ok

10 Testing the cache with the Expires header in the past ... ok

11 Testing the cache with the Expires header in the future ... ok

12 Testing the cache with an Expires header that is overridden ... ok

13 Testing the cache with the Pragma: no-cache header ... ok

14 Testing writing to the cache with non-ASCII data ... ok

15 Testing the cache with the Vary header ... ok

16 Testing the cache with the Vary header and different requests ... ok

17 Testing Capabilities.has_capability with supported capability ... ok

18 Testing Capabilities.has_capability with partial capability path ... ok

19 Testing Capabilities.has_capability with unknown capability ... ok

20 Testing the default values ... ok

21 Testing the multipart form data generation ... ok

22 Testing the encoding of multipart form data with unicode and binary field data ... ok

23 Testing constructing a count resource ... ok

24 Testing constructing an item resource ... ok

25 Testing constructing a list resource ... ok

26 Testing constructing without guessing the resource token ... ok

27 Testing constructing a resource with a specific base class ... ok

28 Testing guessing the resource's token ... ok

29 Testing rewriting of extra_data__ parameters to create ... ok

30 Testing rewriting of exta_data__ parameters to update ... ok

31 Testing item resource fields ... ok

32 Testing item resource link generation ... ok

33 Testing access of a link field ... ok

34 Testing link resource link generation ... ok

35 Testing list resource lists ... ok

36 Testing access of a dictionary field ... ok

37 Testing iterating sub-fields of a dictionary field ... ok

38 Testing generation of methods for the root resource uri templates ... ok

39 Testing ReviewRequestResource.absolute_url with 'absolute_url' field ... ok

40 Testing ReviewRequestResource.absolute_url with generated fallback URL ... ok

41 Testing ReviewRequestResource.absolute_url with 'url' field ... ok

42 Testing BazaarClient diff with file exclusion ... ok

43 Testing BazaarClient diff with file exclusion in a subdirectory ... ok

44 Testing BazaarClient diff with file exclusion in the repo root ... ok

45 Testing BazaarClient diff with changes between a 2nd level descendant ... ok

46 Testing BazaarClient diff with changes only in the parent branch ... ok

47 Testing BazaarClient simple diff case ... ok

48 Testing BazaarClient simple diff with multiple commits case ... ok

49 Testing BazaarClient diff with specific files ... ok

50 Testing BazaarClient get_repository_info with child branch ... ok

51 Testing BazaarClient get_repository_info, no branch ... ok

52 Testing BazaarClient get_repository_info with original branch ... ok

53 Testing BazaarClient guessing summary and description ... ok

54 Testing BazaarClient guessing summary and description for grand parent branch ... ok

55 Testing BazaarClient guessing summary and description with a revision range ... ok

56 Testing BazaarClient.parse_revision_spec with no specified revisions ... ok

57 Testing BazaarClient.parse_revision_spec with one specified revision ... ok

58 Testing BazaarClient.parse_revision_spec with one specified revision and a parent diff ... ok

59 Testing BazaarClient.parse_revision_spec with R1..R2 syntax ... ok

60 Testing BazaarClient.parse_revision_spec with two revisions ... ok