Message-ID: <CAOi1vP8Y6V3mWs1bv=-CCVRfGi59P+3F85n6rzMn0=K6FqQkcw@mail.gmail.com>
Date:   Mon, 19 Sep 2016 13:53:37 +0200
From:   Ilya Dryomov <idryomov@...il.com>
To:     Jean Delvare <jdelvare@...e.de>, Jonathan Corbet <corbet@....net>
Cc:     Ceph Development <ceph-devel@...r.kernel.org>,
        Alex Elder <elder@...nel.org>, Sage Weil <sage@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org,
        Julia Lawall <julia.lawall@...6.fr>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-doc@...r.kernel.org
Subject: "CodingStyle: Clarify and complete chapter 7" in docs-next (was Re:
 [PATCH 03/47] block-rbd: Adjust the position of a jump label in rbd_header_from_disk())

On Mon, Sep 19, 2016 at 11:37 AM, Jean Delvare <jdelvare@...e.de> wrote:
> Hi Ilya,
>
> Sorry for the late answer.
>
> On Tue, 13 Sep 2016 20:31:57 +0200, Ilya Dryomov wrote:
>> Sorry, navigating lkml.org archive is a pain, and I was expecting to
>> see patch.  Your points
>>
>> "The acceptance of an optional single space before labels dates back to
>> at least June 2007, as supported by the very first incarnation of
>> checkpatch.pl. So nothing really new here, except for a preference
>> (my preference, admittedly, but I'm know I'm not alone) being expressed
>> in the coding style document."
>>
>> "Recommendations are not meant to document what people are currently
>> doing but what we think they should be doing."
>>
>> are valid, but note that there is a world of difference between an
>> acceptance and a preference.  The *only* point of whitespace guidelines
>> is to keep the code base consistent.
>
> Consistency is half of the reason, the other half is readability. This
> is why the CodingStyle document has a number of rationales explained.
> This is also why we put whitespace in the first place, while the C
> language doesn't require any ;-)
>
> The sense of my proposal was to address a readability (or usability)
> issue.
>
>> You don't go changing whitespace
>> preferences in such a huge project, not unless you have a *very* good
>> rationale and existing code base is swayed (which it isn't, given the
>> 9/10 ratio).
>
> I did consider the reason to be good enough to warrant a "change",
> actually. Or more exactly from "one space is allowed" to "one space is
> recommended." Which is quite different from changing all the code
> actively. I can understand how you don't like it, but again, this
> "inconsistency" has been accepted for almost a decade now, so I find it
> strange to see so much resistance when someone finally tries to sort it
> out.

Yeah, I guess that's where our disagreement lies - the "so that `diff
-p` does not confuse labels with functions" in the age of git, hg and
others, all of which can be customized to your heart's content is not
a good enough reason to go from "allowed" to "advised".

>
>> >> If I wanted to clarify the
>> >> situation, I'd have gone with "one space indented labels are also
>> >> acceptable" or so.  The example you've re-indented dates back to 2.6.4
>> >> times...
>> >
>> > I can't see how this is relevant.
>>
>> That was a 12 year old example, codifying an existing style used in
>> ~90% cases, serving as a guideline for new contributors.
>
> OK, I get your point now. But the CodingStyle document isn't carved
> into stone. I see 43 changes to that file in recent history (since
> April 2005), some of which are actual changes or clarifications of our
> coding style. This very section of the document was updated in December
> 2014, so not so long ago.
>
> In the end I suppose it boils down to how problematic you consider the
> current situation to be. Apparently you and several other maintainers
> think it's just fine, while me (and a few others apparently) think it
> is not.
>
>> >> git diff also works on regular files, BTW.
>> >
>> > I have no idea what you mean here, sorry.
>>
>> Oh, just that it works outside of git repos too, so you aren't stuck
>> with diffutils if you want to diff two random .c files.
>
> Oh, I had never thought of that. Thanks for the hint :-)
>
>> > (...)
>> > http://marc.info/?l=linux-kernel&m=147325166209844&w=2
>> >
>> > It uses the git diff xfuncname feature you mentioned above. To be
>> > honest I'm surprised it isn't the git default, it seems odd to have so
>> > many diff drivers included in git and not enable them on obvious file
>> > extensions. Oh well.
>>
>> This came up before: http://www.spinics.net/lists/git/msg164216.html,
>> Linus didn't like it.  I suggest you add him to the CC on this patch to
>> see if he changed his mind.
>
> Thanks for the pointer. It is interesting to see many people had been
> bothered by the same problem for many years and even proposed solution
> for it. But also sad to see that nothing happened :-(
>
> Well Linus suggested to improve the default, he was not opposed to the
> change per se I think. But it was 5 years ago and nothing happened
> since then, so I'd rather go with what is available today. Which means
> either one space before labels, or drivers in .gitattributes. Choose
> your poison ;-)

Jon, ping?  My points upthread aside, both CodingStyle and
.gitattributes patches seem to be queued...

Thanks,

                Ilya

Powered by blists - more mailing lists