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

Auto-generate packet dataclasses with Jinja #1374

Merged
merged 1 commit into from
Aug 31, 2024

Conversation

mcm001
Copy link
Contributor

@mcm001 mcm001 commented Jul 16, 2024

Auto-generate our packet serde code. This lets us have a schema hash instead of photon versions.

image

@mcm001 mcm001 changed the title 20240714 serde packet gen Auto-generate packet dataclasses with Jinja Jul 21, 2024
@spacey-sooty
Copy link
Contributor

PhotonSerialisationTest.xlsx
Chucking this here so its easier to find

@mcm001 mcm001 force-pushed the 20240714_serde_packet_gen branch 2 times, most recently from a4a896d to d93c6fc Compare August 4, 2024 04:26
@mcm001 mcm001 marked this pull request as ready for review August 4, 2024 04:27
@mcm001 mcm001 requested a review from a team as a code owner August 4, 2024 04:27
Copy link
Contributor

@spacey-sooty spacey-sooty left a comment

Choose a reason for hiding this comment

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

Does this pr intentionally switch back to pnpm?

@mcm001
Copy link
Contributor Author

mcm001 commented Aug 5, 2024

No, that's weird

Copy link
Contributor

@crschardt crschardt left a comment

Choose a reason for hiding this comment

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

The workflow files use pnpm. This should be switched back to npm before merging this PR.

@mcm001 mcm001 force-pushed the 20240714_serde_packet_gen branch 4 times, most recently from c3d3357 to f81ea12 Compare August 18, 2024 23:35
@gerth2
Copy link
Contributor

gerth2 commented Aug 31, 2024

This is a big one, obviously. I've given it a once over, strategy for generation looks good, and it passes a basic smoke test in Python on my setup

Per discord, we will merge this one and do the deep dive later, as part of the example rework.

@mcm001 mcm001 merged commit 169595e into PhotonVision:master Aug 31, 2024
32 checks passed
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.

4 participants