On Thu, Sep 19, 2013 at 11:53:52PM +0200, David Henningsson wrote:
> On 09/19/2013 01:17 PM, Damir Jeli? wrote:
> > Sorry if I'm a little bit annoying here but I'd like to keep our from
> > now on with as little style inconsistency as possible. I haven't done
> > any proper review/testing on this, just a quick coding style check.
>
> Sorry if I'm even more annoying here ;-) but perhaps you haven't heard
> my personal view on this matter.
>
> I totally disagree. I would instead like to keep our coding style rules
> to a bare minimum to keep the code decently readable. Anything else is
> just an additional road block towards what's important: spending as much
> time as possible fixing bugs and implementing new features, rather than
> spending time complying to coding style rules.
>
> So; for the opening bracket on newline - I think our current coding
> style is wrong and should be changed for functions, and I always forget
> that we have this odd style here. You're okay to comment on that,
> because it is in our coding style rules, but I would appreciate more, as
> you say, "any proper review/testing", rather than a simple coding style
> check. You focus on what looks good on the surface, but what really
> matters is real code quality - i e, whether the code is going to solve
> our users' problems without causing annoying bugs, or not.
>
I understand that a full review is more useful, but a full review also
consists of a 'code style review' and I consider doing only one part of a
review still useful. On the Wayland mailing list everyone is encouraged to
do reviews [1] and doing only one part of a review means that the person that
comes along next doesn't need to do that part as well.
I don't mean to say that I plan to do only coding style reviews on a
regular basis. I only did so this time because you relatively recently
introduced a style inconsistency [2] that slipped through a full review [3].
Yes, I agree, if I have to choose between good style and new features or bug
fixes I would choose the latter, but this is a false dichotomy. We don't
need to choose here.
> > If you're really lazy you can use my sed script to fix this issues (well
> > the opening bracket on newline issues at least).[2]
>
> For the stuff that's *not* in the coding style wiki page, I'm even
> lazier. If you prefer a different style than I happen to write, feel
> free to run your scripts every six months or so and submit a patch for
> it, and I won't complain if somebody else reviews and commit it. (Well,
> unless it severely decreases readability somehow.)
I think you misunderstood me here. I don't demand that you to change anything
about the newlines here and no, I don't plan to be a cleaning lady for our
tree.
Let me rephrase my original mail in a not so 'cold and professional' tone.
The comment about the bracket issue:
'Hey David, please don't forget our coding style.'
The comment about the newlines:
'This looks a little bit odd considering the rest of our tree but do as
you please, or if Tanu has a better idea do as he suggests'
Again sorry if this has annoyed you or if I have worded my thoughts
poorly. I just wanted to make sure that my recent style fixes were not
thrown away effort.
Thanks, Damir.
[1] http://lists.freedesktop.org/archives/wayland-devel/2013-March/008174.html
[2] http://cgit.freedesktop.org/pulseaudio/pulseaudio/commit/?id=de5eb70032344c02d3dfe09ff9d830c16bd595f0
[3] http://lists.freedesktop.org/archives/pulseaudio-discuss/2013-May/017394.html
