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

New codegen architecture #211

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open

Conversation

Virgiel
Copy link
Member

@Virgiel Virgiel commented May 8, 2023

A new codegen architecture generating a whole crate instead of a single file #186.

Changes

  • Generate a full crate instead of a file
  • Inline clients code
  • Minimum amount of dependencies
  • Most features are now automatically enabled, only deadpool remain
  • Split the gigantic generated code file into modules
  • No longer generate a named module for postgres public implicit schema
  • Improve codegen

Current problems

  • Cargo cannot have multiple package with the same name and doesn't support
    nested package. Therefore, I had to use a different name for each generated
    crate.
  • We generate way more code than before, and codegen diff are getting out of control

Remaining improvements

  • Test for types in a schema
  • Update documentation, examples, website, etc

Deferred to future PR

@Virgiel
Copy link
Member Author

Virgiel commented May 13, 2023

@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.

@LouisGariepy
Copy link
Member

@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.

@LouisGariepy
Copy link
Member

Oh I just realized this was still a draft 😅

@Virgiel Virgiel marked this pull request as ready for review May 22, 2023 19:13
@Virgiel
Copy link
Member Author

Virgiel commented May 22, 2023

I still need to add some error handling, but the new architecture is ready for review !

@Virgiel
Copy link
Member Author

Virgiel commented Nov 7, 2023

I've been using this PR in productions for a few weeks and it works very well !

@Virgiel
Copy link
Member Author

Virgiel commented Nov 7, 2023

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 cargo px to generate them on demand. We will continue to check that the codegen works and that the examples run.

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