[FFmpeg-soc] [soc]: r3884 - amr/amrnbdec.c

[Michael Niedermayer]

On Sat, Dec 27, 2008 at 01:34:35PM +0100, Diego Biurrun wrote:
> On Mon, Dec 15, 2008 at 07:59:52PM +0100, Michael Niedermayer wrote:
> > On Mon, Dec 15, 2008 at 02:03:03PM +0000, Robert Swain wrote:
> > > 2008/12/15 Diego Biurrun <diego at biurrun.de>:
> > > > On Mon, Dec 15, 2008 at 01:13:11PM +0100, Benjamin Larsson wrote:
> > > >> Robert Swain wrote:
> > > >> > 2008/12/15 Robert Swain <robert.swain at gmail.com>:
> > > >> >
> > > >> >> 2008/12/15 diego <subversion at mplayerhq.hu>:
> > > >> >>
> > > >> >>> Log:
> > > >> >>> K&R function declaration and whitespace cosmetics
> > > >> >>>
> > > >> >>> --- amr/amrnbdec.c      (original)
> > > >> >>> +++ amr/amrnbdec.c      Mon Dec 15 11:13:50 2008
> > > >> >>> @@ -126,8 +126,9 @@ static int amrnb_decode_init(AVCodecCont
> > > >> >>>
> > > >> >>> -enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size, enum Mode *speech_mode) {
> > > >> >>> -
> > > >> >>> +enum Mode decode_bitstream(AVCodecContext *avctx, uint8_t *buf, int buf_size,
> > > >> >>> +                           enum Mode *speech_mode)
> > > >> >>> +{
> > > >> >>>
> > > >> >> Urgh. I'm happy with the line breaks but I don't tend to like the
> > > >> >> opening { on a new line. I thought that was a GNU thing not a K&R
> > > >> >> thing.
> > > >> >
> > > >> > Nope, it is K&R. Hmm, then who likes them on the same line other than me? :)
> > > >>
> > > >> I do, it is more readable to me. More code per loc.
> > > >
> > > > With that kind of reasoning, we can also prefer
> > > >
> > > >    if (condition) statement;
> > > >
> > > > over
> > > >
> > > >    if (condition)
> > > >        statement
> > > >
> > > > and similar.
> > > >
> > > > But this discussion is completely pointless IMO.  The rules have been
> > > > set in http://ffmpeg.org/general.html#SEC24:
> > > >
> > > >  Indent size is 4. The presentation is the one specified by 'indent -i4
> > > >  -kr -nut'. The TAB character is forbidden outside of Makefiles as is any
> > > >  form of trailing whitespace.
> > > >
> > > > Now it's clear that each person will dislike some part of K&R style and
> > > > prefer to do things in other ways.  But the nature of compromises is
> > > > exactly that: You accept a few things you may not be terribly fond of
> > > > and you get a uniform style in exchange.
> > > 
> > > Mmm. There are a fair few instances of { being on the same line as the
> > > function declaration so I guess you'll have to do those too. I still
> > > don't like it but it is personal preference and if that's what's been
> > > agreed, I won't argue about something like this.
> > > 
> > > If it hadn't been agreed project-wide, I would have preferred to have
> > > been consulted about the changes before they were committed
> > > considering it's my code. Even if I haven't touched it for a while, I
> > > am still active.
> > 
> > i dont remember { placement for functions being discussed or agreed upon.
> 
> Set by Fabrice before your time?  The coding rules are very clear:
> 
>   The presentation is the one specified by 'indent -i4 -kr -nut'. 
> 
> This includes clear rules about brace placement in function definitions.

it does, but
1. as said { placement of functions hasnt been discussed as far as i know
   (you arent saying it has been discussed ...)

2. it hasnt been agreed upon, rather above seems more a guideline choosen
   by fabrice. And in that fabrice did not enforce it AFAIK, so raising
   some fineprint of a complex guidline now to a strict law that should be
   followed letter by letter seems a little strange to me.


> 
> > and i prefer them on the same line as well. Though i am not strongly
> > opposed to following K&R, just that if most people prefer them like we
> > do, that following K&R just because of it would be silly.
> 
> I think following some well-known style is advisable.  Everybody will
> have to make some compromises for this.

following the style that most people working on ffmpeg prefer means fewer
compromises than following one out of 3 well known styles.
Thats simply because the style of fewest compromises is likely not exactly
one of the 3.
Besides, we dont enforce the fineprint of "indent -i4 -kr -nut" anyway
so for this to matter much we first would have to strictly enforce this
and i belive strictly enforcing any style guidline is a bad idea. Simply
because a few good developers have their personal preferances on minor
details of the coding style and are rather emotional about it.
Loosing good developers because of some fineprint in some style seems
a bad choice ...


> 
> Anyway, this is not the best place to discuss this.

no, feel free to start a thread on ffmpeg-dev if the { placement of functions
is that important to you.

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20081227/e21fe93b/attachment.pgp>