-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Create TrackSetTableModel as base for Crate- & PlaylistTableModel #2717
Conversation
872d825
to
16ef907
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for working on this. Here are some comments.
TrackCollectionManager* const pTrackCollectionManager, | ||
QObject* parent); | ||
const char* settingsNamespace); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you like this can be a QString initialized by a QStringLiteral()
Since we store the value finally, you may use a value parameter + std::move() when passing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would have to be changed in quite a few places, so I'd like to refrain from that for now
16ef907
to
1afd4ba
Compare
Since this is already quite big, I'd prefer to merge this as is and then actually implement TrackSetTableModel in a separate PR. @Be-ing could you merge? |
I hope we can finally merge now :) Follow-up tasks:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Holzhaus his comments are resolved :) |
No description provided.