Re: [PATCH v2 2/2] scripts/checkpatch.pl: rebase on top of upstream v5.0-rc6

From: Antony Pavlov <antonynpavlov@gmail.com>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: barebox@lists.infradead.org
Subject: Re: [PATCH v2 2/2] scripts/checkpatch.pl: rebase on top of upstream v5.0-rc6
Date: Mon, 4 Mar 2019 10:26:56 +0300	[thread overview]
Message-ID: <20190304102656.33754174e40b8cfa63f2d28a@gmail.com> (raw)
In-Reply-To: <7475eecb-5e8e-a827-f04e-b3d8d749edaf@pengutronix.de>

On Tue, 26 Feb 2019 10:55:40 +0100
Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

> Hello Antony,
> 
> On 20/2/19 08:14, Antony Pavlov wrote:
> > On Tue, 19 Feb 2019 15:16:47 +0100
> > Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> > 
> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> >> index 4e17347a8481..48b39fbf962a 100755
> >> --- a/scripts/checkpatch.pl
> >> +++ b/scripts/checkpatch.pl
> > ...
> > 
> >> @@ -1555,13 +2997,9 @@ sub process {
> >>  
> >>  			my @compats = $rawline =~ /\"([a-zA-Z0-9\-\,\.\+_]+)\"/g;
> >>  
> >> -			# linux device tree files
> >> -			my $dt_path = $root . "/dts/Bindings/";
> >> +			my $dt_path = $root . "/Documentation/devicetree/bindings/";
> > 
> > At the moment it looks like barebox uses both paths ("/dts/Bindings/" and "/Documentation/devicetree/bindings/")
> > to store dt-related documentation.
> 
> Missed this one. I can reinstate it in a v2. I think I should've caught all barebox specifics now.
> 
> > 
> > The patch is very long and very hard to review.
> 
> Any suggestion on a better way to do it? It's a straight copy from upstream with
> some barebox specific changes applied on top, so I assume ensuring the barebox
> changes are accounted for are all the review we need.

I propose to port checkpatch-related patches from linux one by one.
Of course you can join some patches into one please remember this
quote from https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#split-changes

   The point to remember is that each patch should make an easily understood
   change that can be verified by reviewers. Each patch should be justifiable
   on its own merits.

> 
> I could for v2 include a scripts/checkpatch.patch which patches the corresponding
> upstream checkpatch.pl into the barebox checkpatch.pl. That way reviewing would work
> like this:
> 
> - review checkpatch.patch
> - $ patch -R < checkpatch.patch
> - $ diff $LINUX/scripts/checkpatch.pl $BAREBOX/scripts/checkpatch.pl
> 
> What do you think?

I suppose that we want to get new checkpatch features/bugfixes from linux kernel
but not minimize barebox checkpatch vs linux kernel checkpatch diff size.

-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox