lkml.org 
[lkml]   [2016]   [Sep]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 03/47] block-rbd: Adjust the position of a jump label in rbd_header_from_disk()
    On Tue, Sep 13, 2016 at 4:36 PM, Jean Delvare <jdelvare@suse.de> wrote:
    > Hi Ilya,
    >
    > Thanks for adding me.
    >
    > On Tue, 13 Sep 2016 11:16:13 +0200, Ilya Dryomov wrote:
    >> On Tue, Sep 13, 2016 at 10:12 AM, SF Markus Elfring
    >> <elfring@users.sourceforge.net> wrote:
    >> >>> @@ -1064,7 +1064,7 @@ static int rbd_header_from_disk(struct rbd_device *rbd_dev,
    >> >>> header->snap_sizes = snap_sizes;
    >> >>>
    >> >>> return 0;
    >> >>> -out_2big:
    >> >>> + out_2big:
    >> >>> ret = -EIO;
    >> >>> kfree(snap_sizes);
    >> >>> free_names:
    >> > …
    >> >> Can you point where this current convention is documented?
    >> >
    >> > Yes.
    >> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/Documentation/CodingStyle?id=865a1caa4b6b886babdd9d67e7c3608be4567a51
    >>
    >> Huh. That patch is not in Linus' tree.
    >>
    >> >
    >> > Do you find the software update "CodingStyle: Clarify and complete chapter 7" interesting?
    >> >
    >> >
    >> >> Certainly not in CodingStyle, AFAICT...
    >> >
    >> > I suggest to look at the current version once more.
    >> >
    >> >
    >> >> I know some people prefer a single space in there because it makes
    >> >> "diff -p" work better, but nowadays with "git diff" this argument is
    >> >> pretty moot.
    >> >
    >> > Would you like to discuss the corresponding software evolution a bit more?
    >>
    >> Jon, could you please yank 865a1caa4b6b ("CodingStyle: Clarify and
    >> complete chapter 7") from your linux-next branch or at least change "It
    >> is advised to indent labels" to something less stronger? It hasn't
    >> even hit mainline yet and we are already getting spammed.
    >
    > The problem isn't the documentation update nor whether you or me like a
    > space before labels or not. The problem is Markus Elfring. The guy just
    > spend his time flooding maintainers with unneeded changes they never
    > asked for. Ignore him and you'll be much better. If he was not flooding
    > you with this, he would find something else :-(
    >
    > When I wrote "It is advised to indent labels with one space", I never
    > meant that all the existing code should be converted that way. I

    Hi Jean,

    That much is clear, however ...

    > expressed a preference, and provided a rationale for this preference.
    > After that, an advice is just that: an advice.
    >
    >> Looks like 9 out of 10 labels are not indented
    >>
    >> $ git grep '^[a-z0-9]\+:' -- *.c | wc -l
    >> 27945
    >> $ git grep '^ [a-z0-9]\+:' -- *.c | wc -l
    >> 2925
    >
    > Your regexps are wrong ;-) but the ratio is correct.

    ... one of the main points of any coding style is consistency. When
    someone new wanting to submit say a new driver opens CodingStyle and
    sees "It is advised to indent labels ...", they might start indenting
    labels in their code and advise others to do the same. Given the 9/10
    existing ratio, that advice is wrong. 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...

    >
    >> so I'd say that's a bad advise as far as consistency goes, and the
    >> "diff -p" argument is pretty moot nowadays.
    >
    > It wasn't moot when I sent the documentation update patch. Or why would
    > you think it was? "git diff", by default, behaves exactly the same as
    > "diff -p" with regards to unindented labels (i.e. it doesn't handle
    > them properly.)

    The git diff xfuncname incantation is a few years old now. git diff
    also works on regular files, BTW.

    >
    > However, since then the issue was discussed somewhere else:
    > https://lkml.org/lkml/2016/9/5/214
    >
    > As you can see, alternatives to indenting labels with one space were
    > found. Therefore you will soon be correct saying "the diff -p argument
    > is pretty moot." As soon as my patch hits mainline, actually. Which
    > shouldn't take too long as Andrew Morton picked it 4 days ago.
    >
    > Once this happens, I'm fine with CodingStyle being updated again to
    > reflect the current situation.

    I'm not sure which patch you are talking about - the message you linked
    is not a patch and it's impossible to follow large threads on lkml.org.

    Thanks,

    Ilya

    \
     
     \ /
      Last update: 2016-09-17 09:59    [W:4.052 / U:0.280 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site