Skip to content
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

Various fixes in tools/ to improve code generation #199

Merged
merged 4 commits into from
Oct 3, 2022
Merged

Various fixes in tools/ to improve code generation #199

merged 4 commits into from
Oct 3, 2022

Conversation

pallasscat
Copy link
Contributor

@pallasscat pallasscat commented Oct 3, 2022

Fixed formatting issues (double spaces, excessive newlines, const names) in template output; fixed directory issues and cleaned up code in gen_script.sh; switched to int-based schema version selection and re for code cleanup, minor fixes in generate_from_schema.py.

…into current directory; updated schema version to latest; minor fixes
… to int-based schema version selection; minor fixes
@pallasscat pallasscat marked this pull request as draft October 3, 2022 10:37
@pallasscat pallasscat marked this pull request as ready for review October 3, 2022 10:37
@pallasscat
Copy link
Contributor Author

pallasscat commented Oct 3, 2022

Additionally: do we still need the full template? For example, UnmarshalJSON() method is unnecessary more often than not.

Alternatively, how about re-writing the entire generate.py + schema.tmpl to render enums and objects exclusively? I could try to do it with python3.5+, but it has been years since I switched to golang even for scripting.

@stmcginnis
Copy link
Owner

Alternatively, how about re-writing the entire generate.py + schema.tmpl to render enums and objects exclusively? I could try to do it with python3.5+, but it has been years since I switched to golang even for scripting.

That sounds reasonable. These scripts were left over from when the project was first started and probably are due for a complete overhaul. I would be fine if you want to make some larger changes.

Thanks a lot for looking into this!

@stmcginnis stmcginnis merged commit 5fb1a98 into stmcginnis:main Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants