Em Thu, 26 Sep 2024 10:35:34 +0000 Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> escreveu: > > > Yet, even if the committer did an honest handling of the patch, you may > > > still disagree or want some changes at the original patch. On such cases, > > > the maintainers may decide to drop the changes and do a normal review > > > process. They may otherwise request a patch on the top of the applied > > > one to address the pointed issues. > > > > Let's do a revert in that case, and keep rebases for cases where having > > content in the git history causes issues other than bisection problems. > > I'd very much prefer this as well: revert or fix, if at all reasonable, > instead of rebasing should be a rule. As I answered already, the only rule is that such decision is up to the subsystem's maintainers, as they'll need to handle the consequences of upstreaming broken/reverted stuff. > > I'd argue that even a missing SoB may not be a cause for rebase if it's > > an accident, but that's not worth debating because CI will make sure > > this never happens. > > Does it? > > checkpatch.pl checks should just be warnings. And that should probably > stay. Sob: and From: being different isn't necessarily that far-fetched as > having an address in .mailmap may change From: field but not Sob:, > resulting in a checkpatch.pl warning. > > I wonder if checkpatch.pl should know about .mailmap actually, currently it > doesn't. I could send a patch. It does. Basically, if the media-ci does its work well, we shouldn't have such cases in practice. That said, I guess the logic may require some changes, as there are some complex rules with regards to patches developed by multiple authors. Basically, patches shall follow an exact order of SoBs and tags to indicate other authors for patches with multiple authors. Not sure if CI or checkpatch enforces it currently. > > > There is also worse case scenarios, like a committer violating the > > > committer's agreement. > > > > I'm fine with rebases if someone gets rogue and merges malicious code, > > or commits with insults in the commit message. I don't foresee that > > happening regularly, if ever. > > I'm more concerned of a malicious actor getting access to the committer's > credentials rather than the committer him-/herself going crazy. And if this > happens, changes are it won't be noticed immediately. This could happen as well. From subsystem's maintainer perspective, they both will look the same when checking if a merge is ok. > > Reminding of > <URL:https://github.com/lfit/itpol/blob/master/linux-workstation-security.md> > in the instructions might not be a bad idea. Good point. We're writing a documentation about the new process. Once done, we'll post at media ML for review. Please reply to it if we forget adding it. Thanks, Mauro
