Python 3 #39
Python 3 #39
Conversation
+ travis.yml
Tests can still be run against Python3 with: `tox -e py34`
| @@ -16,6 +16,7 @@ See the **Get Involved** section at the end of this readme to see the current st | |||
| - Unix-based OS, such as Mac or Linux (Windows support is unclear at this time.) | |||
| - [git](http://git-scm.com/) | |||
| - [Python 2.7](https://www.python.org/download/releases/2.7/) | |||
| or [Python 3.4](https://www.python.org/downloads/release/python-343/) | |||
There was a problem hiding this comment.
If tox and Travis are using 3.6, want to bump this?
| @@ -1,10 +1,13 @@ | |||
| from abstract import AbstractClient | |||
| from __future__ import absolute_import | |||
| from .abstract import AbstractClient | |||
There was a problem hiding this comment.
Since we're doing this, could we just use the full paths to the imports?
There was a problem hiding this comment.
Probably? This came from the earlier Python 3 PR, so it's not code I wrote. I'll take a look, though
| from abstract import AbstractClient | ||
| from __future__ import print_function | ||
| from __future__ import absolute_import | ||
| from .abstract import AbstractClient |
| @@ -1,3 +1,6 @@ | |||
| from __future__ import print_function | |||
| from __future__ import absolute_import | |||
| from builtins import object | |||
There was a problem hiding this comment.
Does it work that these imports are above the #! and coding lines?
| from .parser import Parser | ||
| from .commit_parser import CommitParser | ||
| from .terms_collector import TermsCollector | ||
| from .clouseau_model import ClouseauModel |
| @@ -1,3 +1,4 @@ | |||
| from builtins import object | |||
There was a problem hiding this comment.
Same as above re: placement above the #!.
|
|
||
| import os | ||
| import sys | ||
| import re | ||
| import subprocess | ||
| import pprint | ||
| from clouseau_model import ClouseauModel | ||
| from .clouseau_model import ClouseauModel |
| @@ -25,7 +28,7 @@ def parse( self, terms, repo, revlist, clouseau_model, **kwargs ): | |||
| for rev in revlist.strip().split(' '): | |||
| output = self.get_commit(git_dir, rev) | |||
| if output.strip() == '': | |||
| print "WARNING: No output was returned from git for commit [%s]. Ensure the commit exists" % rev | |||
| print("WARNING: No output was returned from git for commit [%s]. Ensure the commit exists" % rev) | |||
There was a problem hiding this comment.
Could we use .format() string formatting? From there it's not a huge jump to f-strings when we go python 3.6+ only 😈 .
| @@ -38,7 +41,7 @@ def get_commit(self, git_dir, commit): | |||
| git_show = subprocess.Popen(git_show_cmd, stderr=subprocess.PIPE, stdout=subprocess.PIPE) # , cwd=git_dir | |||
| (out,err) = git_show.communicate() | |||
| if err: | |||
| print "ERROR running git command [%s]: %s" % (git_show_cmd, err) | |||
| print("ERROR running git command [%s]: %s" % (git_show_cmd, err)) | |||
There was a problem hiding this comment.
Same as above re: string formatting.
| except UnicodeDecodeError: | ||
| line = unicode( line, 'latin-1' ) | ||
| line = str( line, 'latin-1' ) |
There was a problem hiding this comment.
More a comment than anything specific to this PR, this seems like an incredibly fragile way to test encoding.
This incorporates work that @sking963 submitted in #35 (thank you! belatedly!), some tox additions I made in #34, and some tweaks to simplify subprocess handling (Python 3.6 was complaining about how the existing code attempted to join bytes and a string).