[PATCH] Phonon Gstreamer backend

Andre Magalhaes andrunko at gmail.com
Fri Jan 18 17:09:04 GMT 2008


On Jan 18, 2008 1:05 PM, Thiago Macieira <thiago at kde.org> wrote:
> Andre Magalhaes wrote:
> >What the patch does:
> > - Add #include <phonon/phonon_export.h> to common.h and #include
> >"common.h" to all header files.
>
> Why not #include <phonon/phonon_export.h> instead?
>
> There was only one header that missed it.
Nope, no header file was including phonon/phonon_export.h and as
everybody uses common.h I believe it's better to include it in all
header files

> > - Include local headers first (#include "" first then #include <>)
>
> Unnecessary change, but ok. Do it in a separate patch.
>
> > - change all #ifndef FILE_H in the header files to #ifndef
> >Phonon_GSTREAMER_FILE_H
>
> Good point, but separate patch.
>
> > - Do not indent classes definition inside namespaces (almost all
> >files were correct)
>
> Don't do that.
Why? Shouldn' t the code in the same module have the same code standard?
But ok, i can live with that.

> > - Install phonon_gstreamer.desktop
> > - Proper export the plugin factory
>
> Good point, but make that a third patch.

I will split the patch in the following patches:
1) Include phonon/phonon_export.h in common.h and common.h in all
header files (it should compile after that).
2) Install phonon_gstreamer.desktop and proper export the plugin
factory (it should work after that)
3) Proper standardize the #ifndef directives in the header files

BR

-- 
Andre Moreira Magalhaes (andrunko)
--------------------------------------------------------
Jabber: andrunko at gmail.com
MSN:   andremoreira at msn.com
Skype:  andrunko
Blog:    http://andrunko.blogspot.com




More information about the kde-core-devel mailing list