[pulseaudio-discuss] [PATCH 1/4] alsa: Add extcon (Android switch) jack detection

[David Henningsson]

On 09/21/2013 01:21 PM, poljar wrote:
> 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. 

Let me just ask for clarification about the sentence above. When you
submitted patches a while ago, I remember coding style patches changing
things that were not in our coding style rules, e g, you changed every
two newlines into one.

With that patch, you increased the coding style consistency. It sounds
like you want to enforce those changes by adding more coding style
rules; and that's what I'm turning against: I'd like fewer coding style
rules, not more of them.

>>> 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.

Fair point. I just get the feeling that sometimes people do choose the
"only coding style check" type of review just to feel they've done
*something*, whereas the full review is what's really needed.

Or people that can do a full review, might look at the patch and see
that it has already been answered to, and think that maybe they should
review something else instead.

>>> 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.

No worries, no offence taken, and I admit being a bit tired when I wrote
back to you.

-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic