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

implement DeriveIden in sea-orm only #1740

Merged
merged 12 commits into from
Jul 13, 2023
Merged

implement DeriveIden in sea-orm only #1740

merged 12 commits into from
Jul 13, 2023

Conversation

darkmmon
Copy link
Contributor

@darkmmon darkmmon commented Jul 6, 2023

PR Info

  • Dependencies:
  • Dependents:

New Features

  • Using DeriveIden now does not require dependency on sea-query

Bug Fixes

Breaking Changes

  • DeriveIden now has a different implementation.

Changes

@darkmmon darkmmon marked this pull request as ready for review July 11, 2023 10:42
@billy1624 billy1624 marked this pull request as draft July 12, 2023 07:05

assert_eq!(Book::Id.to_string(), "id");
assert_eq!(Book::Title.to_string(), "turtle");
assert_eq!(Book::Text.to_string(), "te_xt");
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be...?

Suggested change
assert_eq!(Book::Text.to_string(), "te_xt");
assert_eq!(Book::Text.to_string(), "TeXt");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think Iden in SeaQuery would make the name to be in snake case, no matter if it is from rename or the original name itself?
Having different behaviour from SeaQuery and SeaORM seems weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh wait
maybe I was misreading the SeaQuery codes...
Seems like it doesn't change it after I re-read the code in SeaQuery.
Will change it back.

Copy link
Member

Choose a reason for hiding this comment

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

Please correct me if I'm wrong. The original behaviour of sea_query::Iden should be...

#[derive(Iden)]
#[iden = "_glyphAbcD"]
struct Glyph;

assert_eq!(Glyph.to_string(), "_glyphAbcD");

@tyt2y3
Copy link
Member

tyt2y3 commented Jul 12, 2023

I want to make sea-query-derive an optional dependency. I will do it in another PR

@plasticbox

This comment was marked as off-topic.

@billy1624
Copy link
Member

Hey @plasticbox, as a temporary workaround, please import the sea_orm::sea_query into scope.

use sea_orm::sea_query::{self};

@plasticbox

This comment was marked as off-topic.

@plasticbox

This comment was marked as off-topic.

@tyt2y3
Copy link
Member

tyt2y3 commented Jul 12, 2023

@plasticbox I believe the build error you saw has been resolved now. If not please open a new thread with details of the error.

Copy link
Member

Choose a reason for hiding this comment

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

This is lovely.

@tyt2y3 tyt2y3 changed the base branch from master to derive_iden July 13, 2023 08:28
@tyt2y3 tyt2y3 marked this pull request as ready for review July 13, 2023 08:28
@tyt2y3 tyt2y3 merged commit 866025a into SeaQL:derive_iden Jul 13, 2023
fnichol added a commit to systeminit/si that referenced this pull request Dec 19, 2023
fnichol added a commit to systeminit/si that referenced this pull request Dec 20, 2023
fnichol added a commit to systeminit/si that referenced this pull request Dec 22, 2023
fnichol added a commit to systeminit/si that referenced this pull request Dec 22, 2023
fnichol added a commit to systeminit/si that referenced this pull request Dec 22, 2023
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.

Depending only on sea-orm, not sea-query, breaks with derive(Iden)
4 participants