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

Update Strum #2088

Merged
merged 8 commits into from
Feb 6, 2024
Merged

Update Strum #2088

merged 8 commits into from
Feb 6, 2024

Conversation

wyatt-herkamp
Copy link
Contributor

Breaking Changes

  • Yes, the strum version might cause some breakage?

@langyo
Copy link
Contributor

langyo commented Jan 31, 2024

I have done some less rigorous testing on my own project, and I can currently confirm that strum 0.26 and strum 0.25 are incompatible with each other. It will affect some modules like sea_orm::DeriveRelation.

@langyo
Copy link
Contributor

langyo commented Jan 31, 2024

cc @tyt2y3

@wyatt-herkamp
Copy link
Contributor Author

I have done some less rigorous testing on my own project, and I can currently confirm that strum 0.26 and strum 0.25 are incompatible with each other. It will affect some modules like sea_orm::DeriveRelation.

Could you please expand upon the issue?

@langyo
Copy link
Contributor

langyo commented Jan 31, 2024

I have done some less rigorous testing on my own project, and I can currently confirm that strum 0.26 and strum 0.25 are incompatible with each other. It will affect some modules like sea_orm::DeriveRelation.

Could you please expand upon the issue?

Here's a part of the code from my own private project:

use chrono::{DateTime, Utc};
use serde::{Deserialize, Serialize};
use strum::{Display, EnumIter, EnumString};

use sea_orm::entity::prelude::*;

#[derive(
    Clone, Debug, PartialEq, EnumIter, EnumString, Display, DeriveActiveEnum, Deserialize, Serialize,
)]
#[serde(rename_all = "snake_case")]
#[sea_orm(rs_type = "i64", db_type = "Integer")]
pub enum Permission {
    #[sea_orm(num_value = 0)]
    Guest,
    #[sea_orm(num_value = 1)]
    User,
    #[sea_orm(num_value = 2)]
    Manager,
    #[sea_orm(num_value = 3)]
    Root,
}

#[derive(Clone, Debug, PartialEq, DeriveEntityModel, Deserialize, Serialize)]
#[sea_orm(table_name = "users")]
pub struct Model {
    #[sea_orm(primary_key)]
    pub id: i64,
    #[sea_orm(default = "now()")]
    pub updated_at: DateTime<Utc>,

    #[sea_orm(indexed)]
    pub permission: Permission,

    pub name: String,
    pub password_hash: String,

    pub email: String,
    pub extra_profile: Option<String>,
}

#[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)]
pub enum Relation {}

impl ActiveModelBehavior for ActiveModel {}

While using strum 0.26 instead of strum 0.25, sea_orm::DeriveEntityModel macro would crash and provide these messages:

the trait bound `models::user::PrimaryKey: Iterable` is not satisfied
the following other types implement trait `Iterable`:
  models::global_config::Column
  models::global_config::PrimaryKey
and 10 others
primary_key.rs(40, 41): required by a bound in `sea_orm::PrimaryKeyTrait`

And the Relation enum will also crashed too that provide these same messages:

the trait bound `models::user::Relation: Iterable` is not satisfied
the following other types implement trait `Iterable`:
  models::global_config::Column
  models::global_config::PrimaryKey
and 10 others
relation.rs(23, 26): required by a bound in `sea_orm::RelationTrait`

I think your patch has fixed this issue that the CI has been succeeded. I would use the new patch version of it when this PR has been merged.

@wyatt-herkamp
Copy link
Contributor Author

wyatt-herkamp commented Jan 31, 2024

This PR should fix it.

CHANGELOG.md Outdated
@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
## Master - Pending

* Added feature flag `sqlite-use-returning-for-3_35` to use SQLite's returning https://github.com/SeaQL/sea-orm/pull/2070
* Update Strum to version 0.26 https://github.com/SeaQL/sea-orm/pull/2088
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for doing this too!

@tyt2y3
Copy link
Member

tyt2y3 commented Jan 31, 2024

Thank you. I just skimmed and looks good. Would invite @billy1624 too.

@tyt2y3 tyt2y3 requested a review from billy1624 January 31, 2024 17:54
Copy link
Member

@billy1624 billy1624 left a comment

Choose a reason for hiding this comment

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

Thanks!! @wyatt-herkamp

@billy1624 billy1624 merged commit 0adcd3b into SeaQL:master Feb 6, 2024
32 checks passed
@wyatt-herkamp wyatt-herkamp deleted the update_strum branch February 6, 2024 11:21
Copy link

github-actions bot commented Aug 4, 2024

🎉 Released In 1.0.0 🎉

Thank you everyone for the contribution!
This feature is now available in the latest release. Now is a good time to upgrade!
Your participation is what makes us unique; your adoption is what drives us forward.
You can support SeaQL 🌊 by starring our repos, sharing our libraries and becoming a sponsor ⭐.

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.

4 participants