-
-
Notifications
You must be signed in to change notification settings - Fork 405
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get tests running on Python 3 #479
Conversation
No. |
@elad661 Why not? |
Because I don't think introducing that dependency is a good idea. It works now, without that. Growing a dependency on a third party library for no reason is a bad thing, especially when it doesn't simplify your code. Not to mention you do the same thing again where you put unrelated stuff in a pull request and this makes it harder for anyone to review your code. I'm not accepting 093b869, and that's final. |
@elad661 Have you tried actually using willie, and its modules, under Python 3? It doesn't work now, because there are mistaken references to Also, tell me what's unrelated in this pull request, and I'll put it out into a separate pull request. Everything I did here was for the express purpose of getting the unit tests to run and pass on Python 3. |
Also, I'm not adding this dependency "for no reason". It does simplify the code, by putting all the 2/3 compatibility in one location rather than making other developers try to figure out what's responsible for what. In addition, you have willie set up as a platform, where other developers can write additional modules that work with willie. By not including a compatibility library like |
I did test willie with Python 3. I'm the one who ported it to python 3. It works. Only with 3.3, so don't bother to test with anything older - we depend on some things that are only present in 3.3 and 2.7. Anyway, no, I won't use six. It just makes the code more complicated, especially since we want to easily STOP supporting Python 2 soon, instead of supporting two versions. praw is an OPTIONAL dependency. You can remove it and willie will still work. If you put six in the core, six will no longer be optional. I don't know why you claim this doesn't work, because it works extremely well here. Instead of making large, untested commits that introduce new dependencies (and most likely break both Python 2 and Python 3. I assume you didn't even bother to test it properly with messages that are not in English, for example), why don't you REPORT THE BUGS YOU ARE SEEING? Commit 093b869 is unrelated to the rest of this pull request because it's about changing how we handle python 3 compatibility and has nothing to do with how the tests are run. If the tests fail after you make it stop complaining about enchant, then we can take a look at this, and see the appropriate solution. I'm not going to merge commit 093b869. Also, your other commits probably won't work because @embolalia reverted your change to setup.py because dev-requirements was missing in the pypi tarball and broke install from tarball. |
@singingwolfboy, as @elad661 said, if you have issues, report them. Contributions are appreciated, but if we don't know what you're trying to solve, we can't check that you are, in fact, solving it. He is also correct about So, as the project's BDFL: |
All unit tests should pass now on python33 with d24c11e. |
This pull requests gets the unit tests to pass while running under Python 3. The code had a bunch of version checks scattered throughout to maintain compatibility -- I took those out and used the
six
library instead, which is designed for that. I also had to modify some of the number formatting in a few finicky tests; the formatting seemed pretty arbitrary, so I didn't think there would be any issue with me modifying it.Once this is merged, can you enable Travis and Coveralls support by signing in with your "embolalia" Github account on their respective websites, and switching on the
willie
repo for each?