-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Replace GORM with GORMv2 #4203
base: develop
Are you sure you want to change the base?
Conversation
@albrechtf Thanks a lot for working on this! I really appreciate it and look forward to taking a look at your changes when I have some time, either tomorrow or when I'm back. I'll only be in the office until Wednesday to get some rest after the release. The GORM v1 magic behind Associations had stopped me out and made me revert all my changes for v2 when I tried to do this myself because I didn't have the time to find a good solution that would work for all existing use cases. I didn't really like the Associations to begin with, but ended up using them anyway so as not to work against the framework and have working software that we can release. We wouldn't be sad at all if the existing Associations could be replaced with something else, as long as we don't have to write lots of manual queries (via copy & paste) instead and the new solution is reliable enough to be released... Similarly, Preload is framework magic that we would like to refactor/replace if this is possible without risking hard to find bugs that we then have to identify and fix before we can release the next stable version. The other changes you mentioned seem straightforward and relatively low risk! 馃憤 |
If you are interested in more detailed feedback, I would be happy to take a look at your code/changes early next week? Anything other than the fixtures you want me to check in particular? Note that, unfortunately, I won't be able to provide you with a ready to use solution/replacement for the Associations and Preload functionality available in Gorm v1. |
You also mentioned an album fixture that has a duplicate UID? If you let me know which one it is, I can quickly resolve that for you. |
I mark this currently as a Draft, although the Go tests look quite good on my end. I will have to do a lot more tests, especially with MariaDB and using the frontend.
Notable changes with GORM v2:
association_
tags. I am really not sure by what to replace some of them. As long as it is just about column naming, replacements arejoinForeignKey
etc., and that was easy to replace.DropTableIfExists
is no longer supported by the Migrator. I replaced it withDropTable
and just a log of any errors.sqlite3
is nowsqlite
in Gorm (sounds like a good idea to me). I only changed the strings (and directory name), but not the constant names.Other notes:
go mod tidy
did for me 馃槃Acceptance Criteria: