Comment 2 for bug 883669

Revision history for this message
RJ Skerry-Ryan (rryan) wrote :

Hey Daniel,

Thanks! This is definitely worth slipping into the 1.10 release.

A few comments:

* I'm still seeing some tabs in your patches -- could you check your editor?

* TRACKMODELCAPS_RELOCATE -- I like where this is going ;)

* getTrack() in ITunesTrackModel/TraktorPlaylistModel/TraktorTableModel did not have similar updates compared to the other getTrack() variants you changed. Was this intentional? I've duplicated what you did to all of them.

* I think we should hold off on over-writing metadata if a Mixxx track already exists for the track that is being getTrack()'d. I just changed your patch so that it does not call all of the setXXX() methods on the Track if it was already in the Mixxx library. This is so that we don't over-write user data if they have done something custom like put the key of the track in the artist field or something. In a future release we can try and figure out how to solve this better.

* I like how you listed every TRACKMODELCAP as commented in getCapabilities() but I think this will go out of date quickly once people add more caps. I removed the commented caps and left comments to look at trackmodel.h for the list.

Committed to lp:mixxx now, thanks!
RJ