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

poljar poljarinho at gmail.com
Sat Sep 21 04:21:42 PDT 2013


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


More information about the pulseaudio-discuss mailing list