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

Create TrackSetTableModel as base for Crate- & PlaylistTableModel #2717

Merged
merged 7 commits into from
May 21, 2020

Conversation

xeruf
Copy link
Contributor

@xeruf xeruf commented Apr 26, 2020

No description provided.

src/library/crate/cratetablemodel.h Outdated Show resolved Hide resolved
src/library/playlisttablemodel.cpp Outdated Show resolved Hide resolved
src/library/tracksettablemodel.h Show resolved Hide resolved
Copy link
Member

@daschuer daschuer left a 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.

src/library/basesqltablemodel.cpp Outdated Show resolved Hide resolved
TrackCollectionManager* const pTrackCollectionManager,
QObject* parent);
const char* settingsNamespace);
Copy link
Member

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.

Copy link
Contributor Author

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

src/library/playlisttablemodel.cpp Outdated Show resolved Hide resolved
@Be-ing Be-ing added this to the 2.4.0 milestone May 7, 2020
@xeruf xeruf force-pushed the refactor-tracksettablemodel branch from 16ef907 to 1afd4ba Compare May 19, 2020 15:28
@xeruf xeruf marked this pull request as ready for review May 20, 2020 10:35
@xeruf
Copy link
Contributor Author

xeruf commented May 20, 2020

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?

src/library/setlogfeature.cpp Outdated Show resolved Hide resolved
src/library/basesqltablemodel.cpp Show resolved Hide resolved
@xeruf xeruf requested a review from uklotzde May 21, 2020 09:51
@xeruf
Copy link
Contributor Author

xeruf commented May 21, 2020

I hope we can finally merge now :)

Follow-up tasks:

Copy link
Member

@Holzhaus Holzhaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would merge but due to the discussion in #2699 I'll wait until @uklotzde has re-reviewed.

@xeruf
Copy link
Contributor Author

xeruf commented May 21, 2020

@Holzhaus his comments are resolved :)

@Holzhaus Holzhaus merged commit 75580cd into mixxxdj:master May 21, 2020
@xeruf xeruf deleted the refactor-tracksettablemodel branch May 21, 2020 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants