Reviewing merge request #141: Nanno playlist: miscellaneous small bugfixes & cleanups

A collection of small bugfixes and cleanups.

It would be good if experts take a look at 2 particular commits: the App.c one, and the PlaylistActions.cpp one.

I think they're likely correct, but this close to release someone should double-check.

Commits that would be merged:

Version 1
  • Version 1
  • 79b94c8
  • Nanno Langstraat
almost 2 years ago
SqlMeta bugfix: setFirstPlayed() set m_lastPlayed.
  • 54b2282
  • Nanno Langstraat
almost 2 years ago
SqlMeta: new score was being calculated based on stale playCount().
  • b195ac6
  • Nanno Langstraat
almost 2 years ago
Playlist bugfix: someone mistook totalLength() for rowCount().
  • b28b276
  • Nanno Langstraat
almost 2 years ago
Playlist bugfix: '-1' should have been '0'. But just get rid of it altogether.
  • d9e06e5
  • Nanno Langstraat
almost 2 years ago
Playlist cleanup: remove strange "1st track" hack in PlaylistActions constructor.
  • beafbb0
  • Nanno Langstraat
almost 2 years ago
Playlist cleanup: fix conflicting setLastPlaying() calls in App.cpp
  • 1683e45
  • Nanno Langstraat
almost 2 years ago
Trivial: some DEBUG_BLOCK statements for performance tracking & easier code flow understanding.
  • 65b4d23
  • Nanno Langstraat
almost 2 years ago
Playlist: add some code comments.
Showing 697707b-79b94c8

Comments

The setFirstPlayed() bugfix was just something I happened to spot while reading the code. But it may actually fix a reported bug: BUG 220895.

avatar Nanno Langstraat | 2010-03-06 11:24:57 UTC

is this still valid? should it still be merged?

avatar Leonardo Franchi | 2010-03-17 22:08:12 UTC

Yes, I think each of these commits is still valid.

avatar Nanno Langstraat | 2010-03-17 22:48:40 UTC

At this point, Nanno himself might possibly the biggest expert we have for playlist code, so I'm not sure who could review this best ;)

avatar Mark Kretschmann | 2010-03-18 09:49:35 UTC

→ State changed from Open to Verifying

Nanno, now that you have push access, could you maybe decide for yourself if this should be merged?

I think that might really be best…

avatar Mark Kretschmann | 2010-03-21 10:37:41 UTC

→ State changed from Verifying to Merged

OK, I just merged it.

I did it in a rebased branch starting at b9060bd3af05b3766ba930609c863c20eb86d942, so it’s easy to revert a single commit if necessary.

avatar Nanno Langstraat | 2010-03-21 13:08:27 UTC

Add a new comment:

Login or create an account to post a comment

How to apply this merge request to your repository