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.

Sign up for GitHub

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

Jump to bottom

virtual: Factor out code generation templates #540

Merged
merged 2 commits into from
Aug 24, 2023
Merged

virtual: Factor out code generation templates #540

merged 2 commits into from
Aug 24, 2023

Conversation

ueno
Copy link
Member

@ueno ueno commented Aug 14, 2023

This makes the current code generation script more generic by factoring out the templates into a separate file.

@coveralls
Copy link

coveralls commented Aug 14, 2023 •
edited
Loading

Coverage Status

coverage: 68.63%. remained the same when pulling fab2dd3 on ueno:wip/dueno/codegen-generic into b6411d7 on p11-glue:master.

@ueno ueno force-pushed the wip/dueno/codegen-generic branch 4 times, most recently from d8334b5 to 26c93ef Compare August 14, 2023 13:18
@ueno ueno marked this pull request as ready for review August 14, 2023 13:19
neverpanic
neverpanic reviewed Aug 14, 2023
View reviewed changes
Copy link
Contributor

@neverpanic neverpanic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate why you chose python source files parsed by the ast module to store the templates, and what advantage that provides over, for example, using YAML, TOML, or JSON?

p11-kit/gen-fixed-closures.py Show resolved Hide resolved
p11-kit/gen-fixed-closures.py Outdated Show resolved Hide resolved
p11-kit/gen-fixed-closures.py Show resolved Hide resolved
p11-kit/gen-fixed-closures.py Outdated Show resolved Hide resolved
p11-kit/gen-wrappers.py Outdated Show resolved Hide resolved
p11-kit/gen-wrappers.py Show resolved Hide resolved
@ueno ueno force-pushed the wip/dueno/codegen-generic branch 2 times, most recently from b8685d5 to 8995c73 Compare August 15, 2023 00:39
@ueno
Copy link
Member Author

ueno commented Aug 15, 2023

Can you elaborate why you chose python source files parsed by the ast module to store the templates, and what advantage that provides over, for example, using YAML, TOML, or JSON?

Since the template values are directly consumed by str.format(), I thought it would be the most natural to use Python literals. The other formats also have drawbacks: YAML (some characters needs different escaping, e.g., a colon), TOML (the parser is part of the standard library only after 3.11), and JSON (no possibility to add comments).

@ueno ueno force-pushed the wip/dueno/codegen-generic branch 5 times, most recently from 7ab421c to 062f6db Compare August 24, 2023 04:32
@ueno
virtual: Factor out code generation templates
3c2575c 
This makes the code generation script more generic by factoring out
templates into separate files.

Signed-off-by: Daiki Ueno <[email protected]>
@ueno
pkcs11-json: Make use of Meson subprojects feature
fab2dd3 
This moves the pkcs11-json submodule under "subprojects", following
the Meson subprojects convention.  This allows the submodule to be
automatically initialized when running meson setup.

Signed-off-by: Daiki Ueno <[email protected]>
@ueno ueno force-pushed the wip/dueno/codegen-generic branch from 062f6db to fab2dd3 Compare August 24, 2023 04:45
ZoltanFridrich
ZoltanFridrich approved these changes Aug 24, 2023
View reviewed changes
Copy link
Contributor

@ZoltanFridrich ZoltanFridrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look fine by me. The threads have been addressed except for one.

@ueno
Copy link
Member Author

ueno commented Aug 24, 2023

Thank you for the reviews!

@ueno ueno merged commit 6b7ffde into p11-glue:master Aug 24, 2023
12 checks passed
@ueno ueno added this to the 0.25.1 milestone Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neverpanic neverpanic neverpanic left review comments

@ZoltanFridrich ZoltanFridrich ZoltanFridrich approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.25.1
Development

Successfully merging this pull request may close these issues.
4 participants
@ueno @coveralls @neverpanic @ZoltanFridrich