Closed longsleep closed 5 months ago
the Identifiable traits gets derived for any table which has a foreign key. That is fine and all but forgets the table the foreign key comes from.
i dont fully understand what it is forgetting; do you mean "forgetting" as in forgetting to add Identifiable
on the foreign struct itself?
i dont fully understand what it is forgetting; do you mean "forgetting" as in forgetting to add
Identifiable
on the foreign struct itself?
Yes. #135 fixes the issue. Users of those structs might expect them to be Identifiable
and make use of this trait in own code.
hey @longsleep, not sure if you're still working with rust (I see some Go in your recent activity), but I wanted to ask for your help in maintaining this repo. Could you join @hasezoey as a maintainer?
Also @hasezoey I saw your name pop up in some steam forums haha. I was really delighted to see and recognize a familiar avatar :hugs: . Thanks for looking out for PRs and merging them in. I saw your comment regarding branch protection rules. Do you want me to remove those?
I saw your comment regarding branch protection rules. Do you want me to remove those?
i mean i understand and think they are OK rules, though they only really work if there is another active pair of eyes that can approve (and has write-access)
hey @longsleep, not sure if you're still working with rust (I see some Go in your recent activity), but I wanted to ask for your help in maintaining this repo. Could you join @hasezoey as a maintainer?
I don't think that I am exactly the right guy as my use case is just a fraction of the project's scope described in the README (For example I have no clue about create-rust-app) means only familiar with a fraction of the code/features in dsync.
I already maintain a lot of things (mostly non-public though, see https://gitlab.com/longsleep) my time is limited. It might still make sense to add me as a maintainer if it helps if I would share my opinion every once in a while (like to get things committed because of merge rules). If this works good enough depends on the volume of changes and the frequency.
Closing this since it was fixed via #135
(For example I have no clue about create-rust-app)
@longsleep -- if it's just that, then I'm not worried :) dsync
was initially built for create-rust-app, but it's actually for diesel codegen in general. I'm hoping we keep the codebase simple / easy for new contributors to jump into and help develop.
volume of changes and the frequency
I remember both @hasezoey and I both believed this crate reached a stable state. I'm still hoping to experiment with advanced queries (for relationships, for example) but I don't think that will be anytime soon.
I'll add you to the maintainers list! Let's move forward with the review rules intact and revisit this if it's still an issue.
In https://github.com/Wulf/dsync/blob/a44afdd08f4447e367aa47ecb91fae88b57f8944/src/code.rs#L214 the
Identifiable
traits gets derived for any table which has a foreign key. That is fine and all but forgets the table the foreign key comes from.A super simple fix goes like this
When a table has a primary key that usually should mean that
Identifiable
should work. Dsync already generates the correct other fields related to this trait at other places. See https://docs.diesel.rs/2.0.x/diesel_derives/derive.Identifiable.html.The above patch works for me. This issue can be used for discussion in case I missed something, before I make a PR for this.