-
Notifications
You must be signed in to change notification settings - Fork 161
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
Add python3 support for building the lib. #66
base: master
Are you sure you want to change the base?
Conversation
@abusi 👍 |
fix all an error |
@swolchok @gjtorikian could we move forward with this pull-request? |
@tsunammis Why are you pinging me? I’m not a maintainer on this project, please don’t do that. |
@gjtorikian You are the second contributor, it was just a ping. Sorry. |
(Sorry for the delay in responding!) Isn't it possible to write code that works under both Python 2 and Python 3? I don't mind supporting Python 3, but I would rather not duplicate the generator code in order to provide that support. |
Yep, it is possible to do so. I wanted to be the less intrusive possible. But yeah i can modify the generator to be python2 and python3 compatible. |
@swolchok Done, now the ast/*.py file are runable by python2 or python3. |
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.
Seems pretty straightforward to me. I would like to see some kind of sanity test that can run if python2 and python3 are available to verify that the same output is produced in both cases; that should guard us against both output divergences and also breaking compatibility with one or the other version.
|
||
def start_union(self, name): | ||
print ('type %s = ' % name), | ||
print(('type %s = ' % name), end=' ') |
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.
shouldn't this be end=''
? (empty string)
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 choose to let 2to3.py do its magic and simply correct incompatibilities.
But yeah, this one is strange, I'll look it up.
@@ -53,6 +53,7 @@ def print_ast(lang_module, input_file): | |||
lang_module.end_file() | |||
if __name__ == '__main__': | |||
import sys | |||
|
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.
nit: remove extra blank line
@swolchok I'll add diff tests with python2 and python3. |
Any update on this PR? |
Yeah, sorry, i'm currently on holydays, i'll finish it when i'll be back, in a week or two. Sorry for the delays. |
Hi @bobh66, i cannot say that i'm maintaining a python3 fork, it's just that I stop the project from requering python 2. As i don't have any use of the 'ctypesgen'erated files, cause i use cffi, i just need the lib to built on an env whithout python2. |
The missing Python 3 support is blocking the inclusion of Tartiflette into the Fedora Package Collection as libgraphqlparser is a dependency. Would be nice if that could be resolved any time soon. |
Hello,
The idea behind this is to have libgraphql not requiring python2 for file generation but also work with python3.
CMakeLists.txt is modified in order to ask for
at least
python 2 and notonly python 2
ast/*.py modified in order to support python2 or python3.
Thanks