Skip to content

Add python3 and tox support#40

Merged
sebdah merged 1 commit intosebdah:masterfrom
jAlpedrinha:enhancement/add-python3
Jan 18, 2016
Merged

Add python3 and tox support#40
sebdah merged 1 commit intosebdah:masterfrom
jAlpedrinha:enhancement/add-python3

Conversation

@jAlpedrinha
Copy link

  • Added support for python3
  • Added test for multiple python versions

@jAlpedrinha jAlpedrinha force-pushed the enhancement/add-python3 branch 14 times, most recently from 584f7ad to b31b9ae Compare January 14, 2016 16:15
@jAlpedrinha
Copy link
Author

@sebdah Could you review this please?

@jAlpedrinha jAlpedrinha force-pushed the enhancement/add-python3 branch from b31b9ae to f1d2cf6 Compare January 14, 2016 16:25
@sebdah
Copy link
Owner

sebdah commented Jan 18, 2016

@jAlpedrinha: Thanks for the PR. Could you please update the code so that is runs on both Python 2.7 and 3.x?

I'm fine with moving to tox, but as a general feedback (no need to change for this one) is to have one PR per change, to make it easier to merge parts of code and review in isolation.

@jAlpedrinha
Copy link
Author

Thanks @sebdah, I totally respect that, and I'm sorry to mix things. I've added tox because breaking 2.7 support is not an option, and I believe the only way to avoid regression when compatibility with both versions matter is to enforce testing.

It should be working in python 2.7, at least is passing the tests. What's broken ?

@sebdah
Copy link
Owner

sebdah commented Jan 18, 2016

@jAlpedrinha Sorry, my bad. Got distracted by https://github.com/sebdah/git-pylint-commit-hook/pull/40/files#diff-354f30a63fb0907d4ad57269548329e3L3.

I'll merge this PR now! Thanks for your contribution!

@sebdah sebdah merged commit f1d2cf6 into sebdah:master Jan 18, 2016
@sebdah sebdah added this to the 2.1.x milestone Jan 18, 2016
@sebdah sebdah self-assigned this Jan 18, 2016
adamn added a commit to adamn/git-pylint-commit-hook that referenced this pull request May 31, 2016
Text updated to reflect [Add Python 3 support](sebdah#40)
@adamn adamn mentioned this pull request May 31, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants