New signals slots syntax in library#2292
Conversation
|
@daschuer Comments addressed, thank you for the review. |
|
You mean @uklotzde I guess ;-) |
|
Ooops, indeed :) |
|
You need to include util/compatibility.h for QOverload |
|
This PR might introduce many merge conflicts with existing PRs. Most of them should be fairly easy to resolve. But we need to start at some point with this migration. Query to all: Any thoughts or objections? I don't mind to rebase or merge my pending changes. |
|
I do not mind either. |
|
I vote for merge, since it contains no changed logic. |
|
@uklotzde merge? |
| #ifndef COVERARTDELEGATE_H | ||
| #define COVERARTDELEGATE_H | ||
|
|
||
| #include <widget/wlibrarytableview.h> |
|
|
||
| QAbstractItemDelegate* BrowseTableModel::delegateForColumn(const int i, QObject* pParent) { | ||
| QTableView* pTableView = qobject_cast<QTableView*>(pParent); | ||
| WLibraryTableView* pTableView = qobject_cast<WLibraryTableView*>(pParent); |
There was a problem hiding this comment.
Minor: This is a typical situation where redundant types can be avoided by using auto
| m_pNetwork = new QNetworkAccessManager(); | ||
| //connect(m_pNetwork, SIGNAL(finished(QNetworkReply*)), | ||
| // this, SLOT(finishedSlot(QNetworkReply*))); | ||
| //connect(m_pNetwork, finished, |
There was a problem hiding this comment.
Let's delete this dead comment
|
@ferranpujolcamins: I'll resolve the merge conflicts and fix the final issues. |
…gnals-slots-syntax-library # Conflicts: # src/library/library.cpp # src/library/scanner/libraryscanner.cpp
|
@ferranpujolcamins: Ready |
Merge master + fix include directives
|
Thank you very much |
|
LGTM |
4 participants
In this PR I update the code under 'src/library' to use the new signals and slots syntax. All changes are pretty straightforward and done with search and replace and some regex.
The things that deserve more careful review are disconnects and the two places where I used closures.
It's quite a big PR because I don't want to open a flood of PRs with these changes, but let me know if you prefer me to split it into smaller PRs.