Fix rbt post on Python 3.5 (invalid kwarg)

Review Request #10354 — Created Dec. 10, 2018 and submitted

Information

RBTools
master
eb5af81...

Reviewers

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.

  • 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
Description From Last Updated

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

daviddavid

This can use rbtools.utils.encoding.force_unicode

daviddavid

This can use rbtools.utils.encoding.force_unicode

daviddavid

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

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

  3. 
      
DavidCain
  1. 
      
  2. 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

  3. 
      
david
  1. 
      
  2. rbtools/api/decode.py (Diff revision 1)
     
     
     
     
     
     
     
    Show all issues

    This can use rbtools.utils.encoding.force_unicode

  3. rbtools/api/request.py (Diff revision 1)
     
     
     
     
     
     
     
    Show all issues

    This can use rbtools.utils.encoding.force_unicode

  4. rbtools/utils/process.py (Diff revision 1)
     
     
    Show all issues

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

  5. 
      
david
  1. 
      
  2. Show all issues

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

  3. 
      
DavidCain
david
  1. Ship It!
  2. 
      
DavidCain
Review request changed
Status:
Completed
Change Summary:
Pushed to release-1.0.x (f17f709)