Conversation

Krzysztof Kozlowski

Edited 1 year ago
Just reviewed a patch with... four comments coming entirely from my templates. No need to write anything, just hit some templates. Two out of these four comments were for not using in-kernel tools for patch submission/testing (checkpatch.pl and get_maintainer.pl). Eh. :(
2
0
1
@krzk including a link to your LPC talk? ;)
1
0
2

@krzk

Below picture is from one of the public chats from the recent LPC. I've seen similar messages elsewhere from others.

I guess sooner or later we need a checkpatch bot that runs checkpatch on submitted patches and sends the results to the list, then you and others could just reply along the lines of "fix this up, but FWIW, ignoring the second warning is fine in this case"

2
0
1

@kernellogger @krzk making b4 run checkpatch automatically would also be pretty nice

1
0
0

@cas @krzk

Maybe.

But the thing is: I somehow dislike the idea of a checkpatch bot myself as it creates noise, but it solves one important thing that similar check on git forges get right:

The check will run (a) on every patch and (b) everyone including reviewers can easily access the results.

2
0
0
@vbabka Different audience :)
0
0
0
@kernellogger Yep, I am all in for the bots.
Checkpatch is garbage, but it is the only garbage we have, so till someone writes something better, please use it.
1
0
2
@kernellogger @cas I am all in for some way of Git-forges. Not necessarily Github or Gitlab (cloud or self-hosted), but something where setting up workflows/pipelines/CI is trivial. Now, to test patches I need to set up my own CI. Other maintainer needed his own. Other needed one more. And all these CIs are not good enough, because testing patches from Patchwork or mailing list requires few additional steps. I don't want to spend my time on writing CI or CI-like-scripts to interact with Patchwork. I want to write a set of simple rules or commands to check every patch sent (or commit) and output warnings with success/error status.

One can do it easily on a Git forge.
1
0
1

@krzk

Don't worry, I use it. 😄 (But like everyone I occasionally forgot, which is another reason to make this run elsewhere).

"the only garbage we have": yup; the problem is that people can ignore it, otherwise maybe someone would have been bothered by it enough to improve it… 🥴

0
0
0
@kernellogger @cas @krzk I disagree that it's garbage. It's just a tool that can give useful reports for many patches, it just shouldn't be treated as the ultimate always correct tool, and some warnings can be ignored sometimes. The problem is how to communicate this aspect clearly.
0
0
3

@krzk @kernellogger @cas OTOH for CI the forges are another set of tools to learn and adapt into your test lab (which may or may not be on the internet!) - there’s very little of the bits I find take any effort that they’re really dealing with. But that is with a bunch of physical systems in my testing flow, they seem better adapted to things you can do entirely in software.

1
0
0
@broonie @kernellogger @cas Any CI you want to setup requires learning its fundamentals and its tools. The CI from the forges is no different here - it only brings some different tools.
I've been setting up different CI systems (Jenkins, Buildbot, Github and Gitlab) and the forges ones are really easy to start with. Complex things are complex everywhere, so I would not call it as a problem.

Plus you can opt-out and use your own CI, via some git-forge-hooks (if we talk about testing patches during review) or directly by git hooks / git fetch (if we talk about testing applied contributions).
1
0
2

@krzk @kernellogger @cas It’s more that the bits that the forges (or other CI tools) do aren’t super relevant for anything I’m doing - I would have to do the whole opt out via hooks thing which is a bunch of pure overhead and if you do external stuff the UX for anyone else tends to be poor for visibility reasons.

The off the shelf tooling does some things well, but if you’re having to do things that don’t fit naturally it can be a constant pain point.

0
0
0