Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
User-Agent: Cyrus-JMAP/3.5.0-alpha0-448-gae190416c7-fm-20210505.004-gae190416
Mime-Version: 1.0
Message-Id: <a0d1f44a-c8ff-4108-af34-6455b5683262@www.fastmail.com>
In-Reply-To: <CAKXUXMxTnz6edBLpBgqOo6uUiSGm8rULH9P8G24xx2OhP_Yb6A@mail.gmail.com>
References: <20210520015704.489737-1-andrew@aj.id.au>
 <CAKXUXMxTnz6edBLpBgqOo6uUiSGm8rULH9P8G24xx2OhP_Yb6A@mail.gmail.com>
Date:   Thu, 20 May 2021 16:54:37 +0930
From:   "Andrew Jeffery" <andrew@aj.id.au>
To:     "Lukas Bulwahn" <lukas.bulwahn@gmail.com>
Cc:     "Linux Doc Mailing List" <linux-doc@vger.kernel.org>,
        "Dwaipayan Ray" <dwaipayanray1@gmail.com>,
        "Joe Perches" <joe@perches.com>,
        "Jonathan Corbet" <corbet@lwn.net>,
        "Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
        openbmc@lists.ozlabs.org, "Jiri Slaby" <jirislaby@kernel.org>
Subject: Re: [PATCH] Documentation: checkpatch: Tweak BIT() macro include
Content-Type: text/plain
Precedence: bulk


On Thu, 20 May 2021, at 16:28, Lukas Bulwahn wrote:
> On Thu, May 20, 2021 at 3:57 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> >
> > While include/linux/bitops.h brings in the BIT() macro, it was moved to
> > include/linux/bits.h in [1]. Since [1] BIT() has moved again into
> > include/vdso/bits.h via [2].
> >
> > I think the move to the vDSO header can be considered a implementation
> > detail, so for now update the checkpatch documentation to recommend use
> > of include/linux/bits.h.
> >
> > [1] commit 8bd9cb51daac ("locking/atomics, asm-generic: Move some macros from <linux/bitops.h> to a new <linux/bits.h> file")
> > [2] commit 3945ff37d2f4 ("linux/bits.h: Extract common header for vDSO")
> >
> > Cc: Jiri Slaby <jirislaby@kernel.org>
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> 
> Looks sound to me.
> 
> I would prefer a bit of word-smithing the commit message by just
> removing the references:
> 
> So:
> 
> > While include/linux/bitops.h brings in the BIT() macro, it was moved to
> > include/linux/bits.h in commit 8bd9cb51daac ("locking/atomics, asm-generic: Move some macros from <linux/bitops.h> to a new <linux/bits.h> file"). Since that commit, BIT() has moved again into
> > include/vdso/bits.h via commit 3945ff37d2f4 ("linux/bits.h: Extract common header for vDSO").
> >
> > I think the move to the vDSO header can be considered a implementation
> > detail, so for now update the checkpatch documentation to recommend use
> > of include/linux/bits.h.
> >
> 
> And then drop references [1] and [2].
> 
> Andrew, what do you think?

I mostly did this because initially I wrapped the commit message and 
checkpatch spat out errors when it failed to properly identify the 
commit description for [1]. But, leaving the description unwrapped 
inline in the text feels untidy as it's just a work-around to dodge a 
shortcoming of checkpatch.

With the reference style the long line moves out of the way and 
checkpatch can identify the commit descriptions, at the expense of 
complaints about line length instead. But the line length issue was 
only a warning and so didn't seem quite so critical.

While the referencing style is terse I felt it was a reasonable 
compromise that didn't involve fixing checkpatch to fix the checkpatch 
documentation :/

Andrew