[Kde-hardware-devel] PortableMediaPlayer capability

Kevin Ottens ervin at kde.org
Sun Apr 2 16:52:45 CEST 2006


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."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 191 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20060402/2a7fadb2/attachment.pgp 


More information about the Kde-hardware-devel mailing list