Summary

Fix rbt post on Python 3.5 (invalid kwarg)

Review Request #10354 — Created Dec. 10, 2018 and submitted Jan. 17, 2019, 1:14 p.m.

Information

Owner

DavidCain

Repository

RBTools

Branch

master

Bugs

Depends On

Commit

eb5af81...

Reviewers

Groups

rbtools

People

Description

Popen didn't gain the encoding parameter until 3.6:
https://docs.python.org/3/whatsnew/3.6.html#subprocess

On 3.5, universal_newlines is available, but encoding is not:
https://docs.python.org/3.5/library/subprocess.html

With these changes, rbt post works in a Python 3.5.6 environment. Similarly, the test suite passes for me (save 3 svn-related tests that are failing due to not have subversion installed).

I'd be happy to add some new tests if desired, but this also seems small enough in scope that it should be safe as-is.

Testing Done


Used rbt post in Python 2.7, 3.5, and 3.6 with these changes
rbt post works when pushing diffs with Unicode content
Test suite passes locally

Issues

Description	From	Last Updated
Also, can you please fill out the "Testing Done" field to describe what you've done to test this?	david	Jan. 10, 2019, 1:58 p.m.
This can use rbtools.utils.encoding.force_unicode	david	Jan. 10, 2019, 1:58 p.m.
This can use rbtools.utils.encoding.force_unicode	david	Jan. 10, 2019, 1:58 p.m.
I think you'd need to add the six.PY3 and sys.version_info.minor >= 6 check here too.	david	Jan. 10, 2019, 1:58 p.m.

flake8 passed.

JSHint passed.

Checking the bug tracker, it seems this resolves https://hellosplat.com/s/beanbag/tickets/4762/

Any input here?

rbt post continues to be non-functional on Python 3.5 due to this bug, despite rbtools' advertised compatibility with Python 3.5

rbtools/api/decode.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This can use rbtools.utils.encoding.force_unicode
```
rbtools/api/request.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This can use rbtools.utils.encoding.force_unicode
```

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

The issue has been resolved. Show all issues

I think you'd need to add the six.PY3 and sys.version_info.minor >= 6 check here too.

The issue has been resolved. Show all issues

Also, can you please fill out the "Testing Done" field to describe what you've done to test this?

Change Summary:

Use force_unicode, describe testing done

Description:

~		Popen didn't gain the `encoding` parameter until 3.6:
	~	Popen didn't gain the encoding parameter until 3.6:
		https://docs.python.org/3/whatsnew/3.6.html#subprocess

~		On 3.5, `universal_newlines` is available, but `encoding` is not:
	~	On 3.5, universal_newlines is available, but encoding is not:
		https://docs.python.org/3.5/library/subprocess.html

		With these changes, rbt post works in a Python 3.5.6 environment. Similarly, the test suite passes for me (save 3 svn-related tests that are failing due to not have subversion installed).

		I'd be happy to add some new tests if desired, but this also seems small enough in scope that it should be safe as-is.

Testing Done:

	+	Used `rbt post` in Python 2.7, 3.5, and 3.6 with these changes
	+	`rbt post` works when pushing diffs with Unicode content
	+	Test suite passes locally

Commit:

7f9dd5d28f7e23132375fb8c2346c2359f097340

eb5af81746bbe22a2cb390c2728dfc423b689f3e

Diff:

Revision 2 (+14 -5)

Show changes

	rbtools/api/decode.py
	rbtools/api/request.py
	rbtools/utils/process.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-1.0.x (f17f709)