First day with LibraryViewManager

[jmigual]

By the way now that I've seen your last message in the Mixxx devel, forget about the getButtonBar(), getLeftPane() and getRightPane() it must be set*() and the LegacySkinParser must setup and style them.

[daschuer]

Yes, this should be done by the Skin, similar how it is working today.

[daschuer]

IMHO you are on th right track, except the skin interaction.

[jmigual]

Hi Daniel,
I've made some changes if you want to review them

[daschuer]

OK this looks scary, but works, because of the clear below.
How about using qDeleteAll or use smart pointers.

[jmigual]

Ok, I'll use qDeleteAll here a smart pointer is not necessary, he have full knowledge of the pointer behaviour

[daschuer]

It looks like this and its overrides are finally unused :-)

[jmigual]

Yes! And I forgot to remove it 😓

On Sun, Jul 3, 2016, 12:19 Daniel Schürmann notifications@github.com
wrote:

In src/library/libraryfeature.h
#10 (comment):

 virtual ~LibraryFeature();

 virtual QVariant title() = 0;
 virtual QIcon getIcon() = 0;

• // Must be a unique name for each feature, it must be a unique name for each
• // different feature
• virtual QString getFeatureName() = 0;

It looks like this and its overrides are finally unused :-)

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/daschuer/mixxx/pull/10/files/fd67608147f2b65021b6b0e7ce49ddf7a3f9ae47#r69388910,
or mute the thread
https://github.com/notifications/unsubscribe/AGMV00HCev3jAg5TxQrZUrFZuY7Qn-hNks5qR4ycgaJpZM4Ir3iU
.

[daschuer]

There is an issue with your own master branch.
28db1fd
Should not exist.

It looks like it is you intention that your master branch mirrors the mixxxdj/mixxx master branch, but it is not.

The issue starts with e5cb15e
This is not a real issue, since the additional commits seams not to contain code, but we should avoid future null merge commits.

This can be done by resetting your master branch:

git checkout master
git reset upstream/master --hard

You should also remove the latest null merge commit by redo the master merge:

git checkout feature/libraryManager
git reset b0fd7c7
git add src/test/durationutiltest.cpp src/util/duration.cpp
git stash
git merge upstream/master -s ours
git stash pop
git commit -a --amend
git push -f

[daschuer]

It is hard to distinguish m_pPlaylistTableModel from m_playlistTableModel.
How about m_playlistTableModelHash?

Can m_playlistTableModelHash become private?

Since both are protected members the lifetime, should be clear.
Or are they used outside the creating class after the feature is gone?

[jmigual]

Yes m_playlistTableModelHash can become private. It's access is done only in the getPlaylistTableModel function, I'll change it