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

Migrate x/group to orm v1. #15295

Closed
julienrbrt opened this issue Mar 7, 2023 · 6 comments
Closed

Migrate x/group to orm v1. #15295

julienrbrt opened this issue Mar 7, 2023 · 6 comments
Assignees
Labels
C:orm C:x/group T:tech debt Tech debt that should be cleaned up

Comments

@julienrbrt
Copy link
Member

julienrbrt commented Mar 7, 2023

x/group currently uses its own ORM. Migrate x/group to use SDK's ORM.

Almost done, but noticed while migrating that it is indeed a bit weird to have to use two proto structs (one api v1 for ORM, and map them to the gogo proto generated structs). Although you do get used to it pretty quickly.

@julienrbrt julienrbrt added C:x/group C:orm T:Sprint T:tech debt Tech debt that should be cleaned up labels Mar 7, 2023
@julienrbrt julienrbrt self-assigned this Mar 7, 2023
@aaronc
Copy link
Member

aaronc commented Mar 7, 2023

Do you have a WIP branch @julienrbrt ?

@julienrbrt
Copy link
Member Author

Do you have a WIP branch @julienrbrt ?

Not yet pushed, some things are still not correct, but opening a draft PR soon!

@technicallyty
Copy link
Contributor

technicallyty commented Jul 25, 2023

any update on this? its causing trouble for clients integrating ORM and x/group, as both are registering "orm" as a codespace for their errors, causing application panic.

@julienrbrt
Copy link
Member Author

julienrbrt commented Jul 26, 2023

We could, for unblocking you, rename the legacy orm error codespace for v0.50. Feel free to submit a PR for that.

@technicallyty
Copy link
Contributor

technicallyty commented Jul 26, 2023

We could, for unblocking you, rename the legacy orm error codespace for v0.50. Feel free to submit a PR for that.

🫡
#17146

@julienrbrt
Copy link
Member Author

julienrbrt commented Oct 30, 2023

Closing this as group will be refactored another time (during #14403), and we will probably not use orm for consistency with the other modules.
It would be great however that the ORM docs points to real life examples (ie regen modules instead), as the SDK probably will not have any.

@julienrbrt julienrbrt closed this as not planned Won't fix, can't repro, duplicate, stale Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:orm C:x/group T:tech debt Tech debt that should be cleaned up
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants