[DISCUSSION] Handling checkpatch flags and suppression

[gastmaier]

PR Description

The change in this PR highlight the location where the checkpatch rules are applied.
Currently, we have two sets:
A default one to remove some of the noise, it was introduced 8 years ago at 0a42a53#diff-4ed0918c4f3aa9fafdedd591f9e7d512a43c5440a6c9474cc0e541b659e75279R19
(I see that from this list was added, this list should be reviewed:

--ignore PARENTHESIS_ALIGNMENT \ # ?
--ignore CAMELCASE \ # ?
--ignore UNDOCUMENTED_DT_STRING \ # ?
--ignore UNKNOWN_COMMIT_ID \` # due to grafted checkout (ok)

)

And a second in addition intended for huge, internal series, where checking each commit is unviable and the series as is is not intended upstream.

It was also understood that users would run b4 prep --check before submitting any series, per documentation:
https://analogdevicesinc.github.io/linux/contribute.html#b4

But I reckon a big red attention warning with a few steps would be better.

A suggestion, is to use PR labels to tune the CI, for example, if to upstream is present, be more strict, if it is the case, I would like suggestions on the tags, and how they would affect the ci/cd.

PR Type

• Bug fix (a change that fixes an issue)
• New feature (a change that adds new functionality)
• Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

• I have conducted a self-review of my own code changes
• I have compiled my changes, including the documentation
• I have tested the changes on the relevant hardware
• I have updated the documentation outside this repo accordingly
• I have provided links for the relevant upstream lore

[nunojsa]

A suggestion, is to use PR labels to tune the CI, for example, if to upstream is present, be more strict, if it is the case, I would like suggestions on the tags, and how they would affect the ci/cd.

I think "upstream" would be a good (and straight) tag. But as we spoke, we are going the "tag" way, we will also need a "downstream" tag for things not intended to go upstream right? So one of them needs to be present for ci to run right?

[pamolloy]

I'm for enabling everything (within reason) by default. Make a set of exceptions for xlnx-main and a set for oran. Don't make broad exceptions because then new projects will adopt them.

Anything requiring manual intervention will never get done, at least not correctly.

[nunojsa]

Make a set of exceptions for xlnx-main and a set for oran

That's what we have already and got bitten upstream for something that passed CI (likely not the case for oran but xlnx-main we need be somewhat permissive). Arguably, the submitter should have ran b4 prep --check and he would see the issue. But I still think there's some room for improvement. Things me and @gastmaier discussed:

1. Use b4 right away for internal development (for things intended for usptream). CI detects the cover and adapts accordingly.
2. Things intended for usptream should open the PR against one of the upstream mirrors (which should be the case anyways). CI can then adapt accordingly.

So, we can either start enforcing both of the above or just one of them. IMO, they should be the default anyways for upstream contributions and from the PR approval to send the patches it would be just a matter of b4 send --reflect (optional) && b4 send.

Now It might be that someone still preferes plain git send-mail (I would not get it but fair enough). In that case, forcing b4 for internal PRs might be overkill. But option 2. should be easy enough to adopt and get things a bit better prepared for upstream submissions.

Thoughts?