fosstodon.org is one of the many independent Mastodon servers you can use to participate in the fediverse.
Fosstodon is an invite only Mastodon instance that is open to those who are interested in technology; particularly free & open source software. If you wish to join, contact us for an invite.

Administered by:

Server stats:

10K
active users

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. :(

@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"

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

Thorsten Leemhuis (acct. 1/4)

@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.

@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.

@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.

@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).

@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.

@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.

@vbabka @krzk @cas

Communicate this aspect clearly? Maybe just use different words for things that can be ignored and things that normally shouldn't be.

Like "error" and "warning".

Ohh wait, we do that already. 🙃

/me runs and hides

For the record: yes, those two words and some others things checkpatch already does to indicate "this might be something that can be ignored" are apparently not enough. 😟