-
Notifications
You must be signed in to change notification settings - Fork 46
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
feature/easier plugin creation #31
base: beatrice
Are you sure you want to change the base?
feature/easier plugin creation #31
Conversation
Added a plugin_code_generator.py which generates code plugin specific code. The script is called when the cmake command is executed, and generates files in libraries/utilities/include/graphene/utilities/. The generated files look like this: "generated_*file_name_where_the_generated_code_is_included*_*name_for_the_code_part_where_the_code_is_included*.hpp
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.
Looks straightforward and simple enough. Can't really comment on the python code itself.
data_dic = dict() | ||
data_dic['plugin_name'] = parsed_data['plugin_name'] | ||
data_dic['plugin_namespace'] = parsed_data['plugin_namespace'] | ||
data_dic['plugin_project'] = parsed_data['plugin_project'] |
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'm not a python expert - is it really necessary to copy the fields instead of appending parsed_data
directly?
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.
Yeah it would be possible to iterate inside there. But my thought was, we don't have much data hence the speed isn't really necessary here. And like this it's just better separated.
# libraries/app/api.cpp | ||
# libraries/app/application.cpp | ||
|
||
generated_files_root_dir = "libraries/utilities/include/graphene/utilities/" |
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 generated files are not really headers (except generated_api_hpp_include_code.hpp
maybe), therefore they shouldn't be put into the include
directory. Also don't use .hpp
for them, perhaps .hxx
or .cxx
depending on content.
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.
Will change that for more clearance.
But generally would you say this is way of solving this issue? I think it looks kinda weird to do includes in the middle of the code. But I heard in some projects this is common. Still wouldn't call it good practice.
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.
Well, I wouldn't call it beautiful. :-)
But if it gets the job done it's ok I guess.
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.
Yeah, that's okay for me. :)
code generator for plugin related code