-
Notifications
You must be signed in to change notification settings - Fork 92
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
tests/testpython.py: Covert to Python 3 #609
Conversation
Untested. |
tests/testpython.py
Outdated
@@ -1,4 +1,4 @@ | |||
#!/usr/bin/python2 | |||
#!/usr/bin/python | |||
|
|||
import importme | |||
print importme.foo |
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.
This is Python 2 syntax.
I've read 9c57a57 and porting this file to Python 3 makes very little sense to me. I belive this file is deliberately Python 2. |
To clarify: The purpose of the test seems to be a script with python2 shebang. If you change the shebang (and port the script to python 3), the purpose of the test is defeated. The test is skipped when python2 is not available. To fix https://bugzilla.redhat.com/show_bug.cgi?id=2249819 IMHO the Fedora package should filter the automatic dependency out instead. |
Thanks for digging into the code. I never got around to digging into what commit 9c57a57 and commit 006d9a1 were actually trying to accomplish, which was made worse by my almost non-existent knowledge of Python these days. So, thanks for doing that.
Okay! |
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.
If this commit is really needed, then there's a minor typo in the commit message. It should be convert, not covert. :)
I'll close this since it is a python2 specific test. Someday it can just be removed entirely but it costs nothing to leave it there, and |
👍🏻 Thanks for digging! |
See: https://bugzilla.redhat.com/show_bug.cgi?id=2249819
See: https://fedoraproject.org/wiki/Changes/RetirePython2.7