[vlc-devel] [PATCH 1/8] stream_extractor: split joint capability into two

[Rémi Denis-Courmont]

Le lauantaina 18. helmikuuta 2017, 10.30.40 EET Filip Roséen a écrit :
> Hi Rémi,
> 
> Thank you for your comments.
> 
> On 2017-02-18 10:06, Rémi Denis-Courmont wrote:
> > Le torstaina 16. helmikuuta 2017, 22.44.11 EET Filip Roséen a écrit :
> > > +     * The following members shall be populated as specified by the
> > > +     * documentation associated with \stream_t for the equivalent name.
> > 
> > Does this actually work? It looks weird; I´d expect \ref stream_t or
> > such...
> The backslash is actually an unfortunate typo, *doxygen* will
> automatically create a link from "stream_t" on it's own as it
> recognize that this is a type.
> 
> > > +    char const* identifier; /**< the name of the entity to be extracted
> > > */
> > 
> > This is valid but I don´t know why a very few developpers do this. The
> > defacto standard is qualifier before type.
> 
> Not sure what *"defacto standard"* you are referring to,

It is both de jure standard (ISO C uses const type, and so does POSIX), and de 
facto (most code and most devs use that order too).

> For consistancy, I much prefer `char const * const` vs `const char *
> const`; though I do not mind developers writing it differently,
> especially not in a *translation-unit* on its own.
> 
> > >  typedef struct stream_extractor_t stream_extractor_t;
> > > 
> > > +typedef struct stream_directory_t stream_directory_t;
> > 
> > Should be merged with the struct typedef (when possible).
> 
> We should spend some time writing up a coding guideline so that we do
> not have to end up with reviews such as this (where it is quite
> obvious that things boil down to a personal taste).

Taste? We do this when it servers some purpose, such as hiding the structure 
layout in a more private location. Here, it does not.

The reason why we have to many typedef struct a a; is hysterical raisins from 
vlc_symbols.h era.

-- 
雷米‧德尼-库尔蒙
https://www.remlab.net/