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
Make constants file 2/3 compatilbe #937
Conversation
Constants file in now both python2 and python3 compatible.
@@ -1,6 +1,8 @@ | |||
#!/usr/bin/env python2 | |||
# -*- coding: utf-8 -*- | |||
# pylint: skip-file | |||
from __future__ import unicode_literals | |||
from builtins import range |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The builtins library is not included in Python by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sophron Thanks for catching that. I included the future
library in the setup.py
but for some reason it still fails. Any idea why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the builtins
library that is missing.
In overall, I'm against adding a new dependency for the sake of Python v3 compliancy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That dependency will only be temporary. After the code is 2/3
compatible we will remove the future
library and get rid of the boilerplate. This method allows us to still work on the project as opposed to stopping all development and change the code base to python 3
. I opened #426 January of 2017. It is now June of 2018 and we have not converted a single line of code. With only a year and half left on python 2
life support I think it is time to move on to python 3
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the most proper way is to have a specific branch of Wifiphisher that runs on Python 3. When the time comes, we can switch our master to that branch.
But for now, master doesn't need to include these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sophron I'm fine with this but what happens if we add a feature or fix a bug in master
? It will need to be merged in both master
and python3
branch and there might be some differences there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will be merged in master
but periodically the python3
branch should be synced.
Add the futuer library to the list of dependencies.
74f00ac
to
795c5fd
Compare
@sophron I have changed the target of this PR to |
Since python3 branch was added to move the code to python3 the travis must be run on there as well. seealso #937
5142314
to
395d371
Compare
Constants file in now both python2 and python3 compatible.