-
Notifications
You must be signed in to change notification settings - Fork 41
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
New codegen architecture #211
base: main
Are you sure you want to change the base?
Conversation
@LouisGariepy Although it is not completely ready for a merge, the core logic is already there and works. This is already a huge PR in itself, so I deferred some of the work for later. I didn't add any new features or made any logical changes to the generated code. |
@Virgiel Thanks for the awesome work! I'm reviewing the code as I'm writing this. Might take a bit since there's a lot to look at, but what I have seen so far is promising. |
Oh I just realized this was still a draft 😅 |
I still need to add some error handling, but the new architecture is ready for review ! |
I've been using this PR in productions for a few weeks and it works very well ! |
A final pain point is that we generate too much code and it makes the PRs hard to read, I suggest putting the examples and reference codegen in gitignore and relying on |
A new codegen architecture generating a whole crate instead of a single file #186.
Changes
deadpool
remainpublic
implicit schemaCurrent problems
nested package. Therefore, I had to use a different name for each generated
crate.
Remaining improvements
Deferred to future PR
cargo-px
instead ofbuild.rs
for theauto-build
example. Recommend this it by default instead of the cli ?SystemTime
, time or chrono.