On Mon, 20 Nov 2017 08:48:10 -0800
Ben Pfaff <b...@ovn.org> wrote:
> On Mon, Nov 20, 2017 at 11:42:43AM -0200, Flavio Leitner wrote:
> > On Sun, 19 Nov 2017 10:58:11 -0800 Ben Pfaff <b...@ovn.org> wrote:
> > > The other technical issue here is about whether && and || should be
> > > at the beginning or end of a line. The coding style document,
> > > intentionally, does not take an explicit position on this, and I did
> > > not realize that the examples in the document are biased toward
> > > putting them at the beginning of the line. OVS uses a mix of
> > > positions. I personally tend to put them at the beginning of a line
> > > because I have a long history of writing code in the GNU coding
> > > style, which mandates this positioning, but others feel exactly the
> > > opposite (for example I believe that kernel style favors end-of-line
> > > positioning). This is a debate that frankly I don't care much for
> > > and I'd prefer to leave it as a matter to individual taste. (This
> > > is also why the coding style document gives some freedom in the
> > > positioning of /* and */ in comment blocks: there is a range of
> > > viewpoints on this and I think the answer doesn't matter.) If
> > > necessary, I favor adding an explicit statement to the document
> > > saying that lines may be split before or after binary operators.
> > > (On the other hand I do prefer ? and : at the beginning of a line.)
> >
> > It all makes sense except for the freedom in the coding style. Too much
> > freedom means no standard.
> >
> > It is natural that the code evolves and maybe new things will impact on
> > the current coding style guidelines. I agree we don't need to rewrite
> > OVS every time that happens, but as it happened in this case, one could
> > update the style being used in a file at some point, so that the coding
> > style document rules the major portion of the code base.
> >
> > I came from the kernel and yet the patch uses the other style, which
> > I figured was the preferable one. Even with that fossil in me a bit
> > reluctant to adopt mid-block declarations, I honestly don't care as
> > long as I can see a pattern ruling a good portion of the code.
> >
> > Yes, I realize the proposed patch (please drop it) has changes going
> > in the other direction because it never occurred to me that we could
> > update the coding style with regards to where to break lines.
> >
> > How to decide which one is preferable? Maybe take a snapshot of OVS
> > today and see what is mostly used and take from there?
>
> One reason that the coding style takes no position is that I can see an
> argument for each possibility. Positioning a binary operator before a
> line break gives the operands a more consistent look, e.g.:
>
> a = (b |
> c |
> d);
>
> but positioning after a line break better draws the eye to the binary
> operator, which makes it more obvious what's happening, e.g.:
>
> if (a
> && b
> && c)
>
> These days, I personally choose a style depending on which of those
> factors I consider more important in an individual situation.
>
> What do you think?
Both have pros and cons, so I have no definitive answer to each one
is better.
But I find more difficult to read and code when we have mixed styles,
don't you?
Interesting that the kernel style favors the end-of-line positioning,
so if we choose that, OVS would be one step closer to the kernel DP
coding style.
A quick stats shows OVS preference is more towards the other way though.
$ git grep '^[ \t]\+[&|\|]\{2\}' *.c | wc -l
1346 (60%)
$ git grep '[&|\|]\{2\}[ ]*$' *.c | wc -l
865 (40%)
Perhaps it all boils down to the lack of documentation in the coding
style saying what you just said above. Because I was trying to follow
the style in each file, so every time I switch to another file, I have
to stop and find out which style is being used again and again. That
is annoying. Therefore, your criteria helps a lot, because now I know
it's not per project and neither per file, it's about which one I consider
more suitable in each individual situation.
Thanks,
--
Flavio
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
