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

once-common-structs and once-connection-type #80

Merged
merged 3 commits into from
Oct 9, 2023

Conversation

hasezoey
Copy link
Collaborator

This PR adds options once-common-structs and once-connection-type

These options are currently NOT the default

to make them default, should they still be a option or should it be unconfigurable? also i dont think no-once-connection-type or no-once-common-structs sounds good, any suggestions?

re #56

@hasezoey hasezoey requested a review from Wulf September 11, 2023 12:58
Copy link
Owner

@Wulf Wulf left a comment

Choose a reason for hiding this comment

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

Awesome -- yeah, this should definitely be the default and it's a great refactor of the generated output. We can make it the default in another PR

Let's do this for the tests:
Add a second table to the schema and also remove all fields from the tables except maybe an ID, similar to my comment for #82

@hasezoey
Copy link
Collaborator Author

added a second table and rebased onto main

@hasezoey hasezoey requested a review from Wulf September 30, 2023 18:29
Copy link
Owner

@Wulf Wulf left a comment

Choose a reason for hiding this comment

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

It would be nice to add another test for these options + the single-model-file option but if we're thinking of making these the default, it should be okay to ship this!

Again, thanks for this great refactor of the generated code

src/lib.rs Show resolved Hide resolved
@hasezoey
Copy link
Collaborator Author

hasezoey commented Oct 8, 2023

It would be nice to add another test for these options + the single-model-file option

added with 007ace8 (all options together), i didnt have a better name for the folder though

Copy link
Owner

@Wulf Wulf left a comment

Choose a reason for hiding this comment

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

awesome, thank you @hasezoey

@hasezoey hasezoey merged commit 007ace8 into Wulf:main Oct 9, 2023
6 checks passed
@hasezoey hasezoey deleted the once_common branch October 9, 2023 11:43
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