Le Vendredi 31 Mars 2006 20:57, Davide Bettio 'WindowsUninstall' a écrit : > Here is a patch for PortableMediaPlayer capability. > I will document this capability.
IMO you should document and develop the classes in parallel. I don't know for you, but it's always harder when you do it afterward (even if its a few days)... For me it's definitely harder! ;-) > Can I commit this patch? Ok, here are my comments: 1) playlistPath() I suspect this one comes from the HAL spec. ;-) IMO you should keep it out (at least for now). My rationale behind this is that it maps to playlist_path in HAL, and seems useless if you don't map filepath_format and audio_folders. I'd treat them in an "all or nothing" way. 2) ifaces/portablemediaplayer.h file You should add the Q_PROPERTY and Q_ENUMS macro there, but commented out (it somewhat goes with the "document the class" task, until I find a better way to achieve this in the backend implementations (I'm kind of stuck since moc doesn't expand macros, you have to copy this everywhere). Please look at how it's done in ifaces/camera.h for example. 3) The Fake backend is missing Q_PROPERTY and Q_ENUMS 4) The HAL backend is missing Q_ENUMS 5) The HAL backend counterpart of your patch can't work. You forgot the "portable_audio_player" prefix in the property keys. 6) The modifications about Camera in the backends should go in a separate patch/commit just so that it's not lost in history. ;-) Regards. -- Kévin 'ervin' Ottens, http://ervin.ipsquad.net "Ni le maître sans disciple, Ni le disciple sans maître, Ne font reculer l'ignorance."
pgpw2cPdlhKPJ.pgp
Description: PGP signature
_______________________________________________ Kde-hardware-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-hardware-devel
