Refactor: add top-level model module
#1317
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This just makes it work the same as all configuration, which was changed a while ago. Both `TranslatedString` types were combined into one type. This commit also adjusts some docs that have been forgotten. There is a second subtle breaking change: it's only possible to import labels with languages that Tobira knows about, and not arbitrary two- letter codes, like before. I don't expect that to be a problem. Tobira was never able to actually show these in the UI anyway.
This is the start of a restructuring where we move several types that currently randomly live in one submodule, despite interacting with several.
owi92
approved these changes
Jan 20, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the start of some backend refactoring. We noticed for a while that many types don't cleanly fit into one submodule like
db,api, or something else, because they have DB impls, are loaded from the DB, are exposed in the API and potentially other uses. Having a new top level module house these make sense to me. I also don't mind having smaller files in there, with fewer types per file. And I also don't mind having many submodules insidemodule.Note that this doesn't mean that we won't have two types for the same "thing" at all anymore. Sometimes it makes sense to have a separate type! For example, see some
search::*types below which represent exactly what's stored in the search index. We want to disconnect that from the API representation somewhat, they don't necessarily need to match.This PR does not perform the full refactor, far from it! It just adds the module and moves 3 interesting types already. To reduce merge conflicts I expect us to move things around over the next couple months.
This is based on #1316, so that should be merged before.
I went through all modules (except
api) and checked all types. Here are more things I would move in the future:db::typessearch::{TextSearchIndex, TextAssetType, TextMatch}probably into somemodel::text?search::{Event, Playlist, Realm, Series, User}: not sure about that. These are mostly a representation of whats stored in the search index and have a separateSearchFootype inapi::model. I think these can stay. With one exception:search::Playlistcurrently does not have a separateSearchPlaylisttype. It should probably get one at some point.api::model? I haven't really looked at it closely yet.In terms of review: ehm... all the moving around and import changing is annoying to review and I don't really see the point. Maybe roughly look at the new structure? And maybe test a bit? Dunno, or nothing.