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

Add CLI option to sea-orm-cli to skip primary keys with serde on generation of entities #846

Closed
wants to merge 3 commits into from

Conversation

Witcher01
Copy link
Contributor

PR Info

Adds

  • Command line argument --skip-primary-key-deserialization to sea-orm-cli for the option to skip deserialization of primary keys with serde by adding #[serde(skip_deserialization)] to the primary key

As the comment says, this adds the option to skip serializing primary keys with serde in entities generated by sea-orm-cli.
The command line flag name is a bit long, so feel free to suggest a different name for it.
The implementation could use some work to feel better, so I'll take suggestions for this as well.

As of now clap doesn't treat the new argument as only being allowed to be set when --with-serde with a value other than none is used.

@Witcher01 Witcher01 changed the title Add CLI option to skip primary keys with serde Add CLI option to sea-orm-cli to skip primary keys with serde on generation of entities Jul 6, 2022
@Witcher01
Copy link
Contributor Author

Right, I didn't look at the tests. This might take a bit of time

Comment on lines +618 to +626
if skip_pk_deserialization {
quote! {
#[sea_orm(#ts)]
#[serde(skip_deserialization)]
}
} else {
quote! {
#[sea_orm(#ts)]
}
Copy link
Member

Choose a reason for hiding this comment

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

Hey @Witcher01, sorry for the delay! Thanks for the great contributions!! Looks good overall :)

I have a minor nitpick. Please check if the refactoring make any sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries! I'm not sure what you mean, my commit looks good to me, tests run, and running it gives me the right results, too.
Could you elaborate? Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

I don't want to keep a mutable skip_pk_deserialization and that's why refactored the code :P

@Witcher01
Copy link
Contributor Author

Witcher01 commented Aug 3, 2022

I just noticed that skipping the primary key deserialization isn't added in the expanded formats. I'll fix that and push a new commit when I'm done. ETA this evening, hopefully.
I'm also adding tests for this, as I haven't done that yet.

@billy1624
Copy link
Member

Thanks! @Witcher01

@Witcher01
Copy link
Contributor Author

I'm not finding any time to implement this. If anyone else wants to have a go at this with my changes already there, feel free to keep working on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add sea-orm-cli flag to skip deserialization of primary keys for entities
2 participants