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

[Michael Niedermayer]

On Sat, Dec 27, 2008 at 03:20:48PM +0100, Diego Biurrun wrote:
> On Sat, Dec 27, 2008 at 03:03:49PM +0100, Michael Niedermayer wrote:
> > 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.
> 
> Nobody is doing that, the files I edited are not following K&R style
> strictly, before or after my changes.
> 

> You have always treated the coding rules as a binding document

If by coding rules you mean coding style, then i think you are wrong
i did not treat the coding style as strictly binding, rather as a
recommandition that people should follow but that exceptions would be
no problem if someone feels strongly about something.

If OTOH you mean rules related to coding quality or svnlog/history
reviewabilty then yes i did and do consider these rules important and
reject patches if they are not followed.


> even though large parts of it have not been agreed upon, much
> less by most of the developers, some parts it seems, not even
> by you..

If you or someone else thinks some part of it is bad, you can start
a discussion about it on ffmpeg-dev.


> 
> > > > 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.
> 
> I doubt it.

i see

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- Måns Rullgård
-------------- 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/1c590263/attachment.pgp>