[GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?

[GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?

[Jacopo Notarstefano]

It seems to me that the topic of adding the checkpatch.pl script to
Git's source tree has cropped up several times in the past, as
recently as a couple of days ago: $gmane/243607.

It should be noted that its usage for its sake has been discouraged by
Junio Hamano in $gmane/205998. Also, its use is somewhat controversial
and has led to flames and even a public fork.

Despite this, I think that git might benefit from a port of
checkpatch.pl. In fact, even Junio had admitted to use part of its
features later in $gmane/205998.

We could simply use linux's script/checkpatch.pl, but I think a port
is needed for these reasons:

1. Git style guidelines are somewhat different and less strict than
their Linux equivalents.
2. Several patch threads bounce back and forth because of style fixes.
A checkpatch script added as a hook could help reduce these and use
more efficiently our time.
3. As far as I can tell, checkpatch needs to be run from the root
folder of a linux repository clone. Cloning several hundred MBs for a
single perl script looks a little foolish to me.

So, is there any interest in adding a port of checkpatch.pl to
contrib/? I might work on this as part of GSoC. I still haven't
submitted my application about git bisect (life got in the way!), but
Michael Heggarty remarked in $gmane/242703 that my original idea had
too little meat in it to constitute a good GSoC proposal.

Cheers,
Jacopo Notarstefano

Re: [GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?

[Eric Sunshine]

On Mon, Mar 17, 2014 at 9:38 PM, Jacopo Notarstefano
<jacopo.notarstefano@gmail.com> wrote:
> It seems to me that the topic of adding the checkpatch.pl script to
> Git's source tree has cropped up several times in the past, as
> recently as a couple of days ago: $gmane/243607.
>
> It should be noted that its usage for its sake has been discouraged by
> Junio Hamano in $gmane/205998.

In the referenced message, Junio says that he often runs checkpatch.pl
on incoming patches.

> Also, its use is somewhat controversial
> and has led to flames and even a public fork.
>
> Despite this, I think that git might benefit from a port of
> checkpatch.pl. In fact, even Junio had admitted to use part of its
> features later in $gmane/205998.
>
> We could simply use linux's script/checkpatch.pl, but I think a port
> is needed for these reasons:
>
> 1. Git style guidelines are somewhat different and less strict than
> their Linux equivalents.

Are checkpatch.pl's customization options, such as --ignore,
insufficient to make it behave in the desired fashion for git?

> 2. Several patch threads bounce back and forth because of style fixes.
> A checkpatch script added as a hook could help reduce these and use
> more efficiently our time.
> 3. As far as I can tell, checkpatch needs to be run from the root
> folder of a linux repository clone. Cloning several hundred MBs for a
> single perl script looks a little foolish to me.

No need to clone the kernel. checkpatch.pl runs fine standalone with
the --no-tree option.

> So, is there any interest in adding a port of checkpatch.pl to
> contrib/? I might work on this as part of GSoC. I still haven't
> submitted my application about git bisect (life got in the way!), but
> Michael Heggarty remarked in $gmane/242703 that my original idea had
> too little meat in it to constitute a good GSoC proposal.
>
> Cheers,
> Jacopo Notarstefano

Re: [GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> 1. Git style guidelines are somewhat different and less strict than
>> their Linux equivalents.
>
> Are checkpatch.pl's customization options, such as --ignore,
> insufficient to make it behave in the desired fashion for git?

If we are to officially encourage the use of checkpatch.pl, we
should stress that mechanically adhering to its check is *not* the
goal.  As long as people view it as a tool to _help_ them spot
obvious problems before sending their patches out, but the final
responsibility to produce readable code is upon themselves, not the
script, I am personally fine.  Oftentimes we find an occasional long
line that is slightly longer than 80-column limit a lot easier to
read than chopping it artificially in the middle, for example.

>> 2. Several patch threads bounce back and forth because of style fixes.
>> A checkpatch script added as a hook could help reduce these and use
>> more efficiently our time.
>> 3. As far as I can tell, checkpatch needs to be run from the root
>> folder of a linux repository clone. Cloning several hundred MBs for a
>> single perl script looks a little foolish to me.
>
> No need to clone the kernel. checkpatch.pl runs fine standalone with
> the --no-tree option.
>
>> So, is there any interest in adding a port of checkpatch.pl to
>> contrib/?

Not really.  Maintaining the forked version is an additional pain we
do not necessarily need.  Are we going to carry a port of gcc and
others?

I would rather prefer to add a paragraph in CodingGuidelines that
points at the canonical location to obtain the script, e.g.

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/scripts/checkpatch.pl

and a procedure to run the script over their patch.  I am too lazy
to check myself, but there must be somewhere we mention the use of
format-patch and send-email in the document, and use of the script
to check would fall naturally between these two steps, I would
think.

Re: [GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?

[Junio C Hamano]

Jacopo Notarstefano <jacopo.notarstefano@gmail.com> writes:

> It seems to me that the topic of adding the checkpatch.pl script to
> Git's source tree has cropped up several times in the past, as
> recently as a couple of days ago: $gmane/243607.
>
> It should be noted that its usage for its sake has been discouraged by
> Junio Hamano in $gmane/205998.

I've never said any such thing.

I only said I am not enthused against a proposal to add a build
target that runs checkpatch or equivalent over *all* existing code,
which will invite needless churn (read again the part of the message
you quoted before I say "I am not enthused").

It is a totally separate issue for submitters to make sure they do
not introduce *new* problems, and use of "checkpatch --no-tree"
could be one way to do so.

Re: [GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?

[Jacopo Notarstefano]

On Tue, Mar 18, 2014 at 8:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> I've never said any such thing.
>
> I only said I am not enthused against a proposal to add a build
> target that runs checkpatch or equivalent over *all* existing code,
> which will invite needless churn (read again the part of the message
> you quoted before I say "I am not enthused").
>
> It is a totally separate issue for submitters to make sure they do
> not introduce *new* problems, and use of "checkpatch --no-tree"
> could be one way to do so.
>

Sorry for misrepresenting your opinion. My summarization algorithm
sometimes is _very_ lossy :)

Re: [GSoC14][RFC] Is there any interest in adding a port of checkpatch.pl to contrib/?

[demerphq]

On 18 March 2014 02:38, Jacopo Notarstefano
<jacopo.notarstefano@gmail.com> wrote:
> 3. As far as I can tell, checkpatch needs to be run from the root
> folder of a linux repository clone. Cloning several hundred MBs for a
> single perl script looks a little foolish to me.

If that is your worry maybe you should upload the script to CPAN.

Yves


-- 
perl -Mre=debug -e "/just|another|perl|hacker/"