On 23/01/14 10:53, Diego Biurrun wrote:
> On Wed, Jan 22, 2014 at 09:32:22PM +0100, Vittorio Giovara wrote:
>> Ok let's face this beast.
> 
> \o/
> 
>> On 16/gen/2014, at 03:55, Diego Biurrun <[email protected]> wrote:
>>> --- a/libavformat/utils.c
>>> +++ b/libavformat/utils.c
>>> @@ -19,35 +19,35 @@
>>>
>>> +#undef NDEBUG
>>> +#include <assert.h>
>>> +#include <stdarg.h>
>>> #include <stdint.h>
>>>
>>> -#include "avformat.h"
>>> -#include "avio_internal.h"
>>> -#include "internal.h"
>>> -#include "libavcodec/internal.h"
>>> -#include "libavcodec/bytestream.h"
>>> -#include "libavutil/opt.h"
>>> -#include "libavutil/dict.h"
>>> -#include "libavutil/internal.h"
>>> -#include "libavutil/pixdesc.h"
>>> -#include "metadata.h"
>>> -#include "id3v2.h"
>>> +#include "config.h"
>>
>> config.h should be e first header to be included.
>>
>>> #include "libavutil/avassert.h"
>>> #include "libavutil/avstring.h"
>>> #include "libavutil/mathematics.h"
>>> #include "libavutil/parseutils.h"
>>> #include "libavutil/time.h"
>>> -#include "riff.h"
>>> +#include "libavutil/opt.h"
>>> +#include "libavutil/dict.h"
>>> +#include "libavutil/internal.h"
>>> +#include "libavutil/pixdesc.h"
>>> +#include "libavcodec/bytestream.h"
>>> +#include "libavcodec/internal.h"
>>> #include "audiointerleave.h"
>>> +#include "avformat.h"
>>> +#include "avio_internal.h"
>>> +#include "id3v2.h"
>>> +#include "internal.h"
>>> +#include "metadata.h"
>>> +#include "riff.h"
>>> #include "url.h"
>>> -#include <stdarg.h>
>>> #if CONFIG_NETWORK
>>> #include "network.h"
>>> #endif
>>
>> I think you should add a new line between header groups.
> 
> Is that the agreed-upon standard now?

https://wiki.libav.org/CodingStyle/HeaderOrdering

> Doxygen does not require any such thing, no.

It is inconsistent. with the rest and looks worse than 3 lines.

> If they are complete sentences with noun and verb, then I am in favor
> of using proper capitalization and punctuation rules.  In this file I
> was inconsistent, yes.  I just could not be bothered to adjust all of
> them.

Can be done later.

> That's the way that THE BOOK formats most of its casts.

And nothing prevents it to be wrong. Will be discussed in the
post-fosdem sprint.

> What is unsigned supposed to be if not an int?  Do we have opinions from
> others?

Last time such item got discussed there were the tendency to be
extremely clear and use unsigned int. (adding that item to codingstyle
would be good)

> I generally follow the automatic suggestions of emacs when I press TAB
> in these cases, unless the \ should be beyond the 72nd column, which my
> emacs does not move past when I TAB.  For compact blocks like this one
> having the \ so far out looks a tad silly IMO.

Personally I prefer 1 space separation, this item is completely
inconsistent and not discussed so whichever you like is ok IMHO.

Please commit anytime with or w/out the comments above addressed.

lu
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to