Re: RFC: Github PR bot questions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 17, 2021 at 9:24 AM Mauro Carvalho Chehab
<mchehab+huawei@xxxxxxxxxx> wrote:
>
> Em Thu, 17 Jun 2021 08:33:29 -0600
> Rob Herring <robh@xxxxxxxxxx> escreveu:
>
> > On Thu, Jun 17, 2021 at 2:55 AM Mauro Carvalho Chehab
> > <mchehab+huawei@xxxxxxxxxx> wrote:
> > >
> > > Em Thu, 17 Jun 2021 10:20:31 +0200
> > > Dmitry Vyukov <dvyukov@xxxxxxxxxx> escreveu:
> > >
> > > > On Thu, Jun 17, 2021 at 8:53 AM Mauro Carvalho Chehab
> > > > <mchehab+huawei@xxxxxxxxxx> wrote:
> > > > >
> > > > > Em Wed, 16 Jun 2021 15:11:33 -0600
> > > > > Rob Herring <robh@xxxxxxxxxx> escreveu:
> > > > >
> > > > > > On Wed, Jun 16, 2021 at 11:18 AM Konstantin Ryabitsev
> > > > > > <konstantin@xxxxxxxxxxxxxxxxxxx> wrote:
> > > > > > >
> > > > > > > Hi, all:
> > > > > > >
> > > > > > > I've been doing some work on the "github-pr-to-ml" bot that can monitor GitHub
> > > > > > > pull requests on a project and convert them into fully well-formed patch
> > > > > > > series. This would be a one-way operation, effectively turning Github into a
> > > > > > > fancy "git-send-email" replacement. That said, it would have the following
> > > > > > > benefits for both submitters and maintainers:
> > > > > >
> > > > > > What makes this specific to Github PRs? A Github PR is really just a
> > > > > > git branch plus a target at least to the extent we would use it here.
> > > > > > The more of this that works on just a git branch, the more widely
> > > > > > useful it would be.
> > > > > >
> > > > > > > - submitters would no longer need to navigate their way around
> > > > > > >   git-format-patch, get_maintainer.pl, and git-send-email -- nor would need to
> > > > > > >   have a patch-friendly outgoing mail gateway to properly contribute patches
> > > > > >
> > > > > > Presumably, the bot would rely on get_maintainer.pl or it would get
> > > > > > who to send to based on GH repo and reviewers? Without work on
> > > > > > get_maintainer.pl, I don't think it will work well beyond simple
> > > > > > cases.
> > > > >
> > > > > Some sanity test is needed, as otherwise it will end by trying to send
> > > > > the patch to a large number of people.
> > > >
> > > > I think this system needs to use get_maintainer.pl results as is and
> > > > any fixing/filtering/sanity checking needs to go into
> > > > get_maintainer.pl itself.
> > > > get_maintainer.pl is what is used by lots of contributors, the only
> > > > option for any automated systems, what is used by new contributors if
> > > > they don't use this system anyway. And even experienced developers
> > > > know internal rules only for a few subsystems and use
> > > > get_maintainer.pl when sending a one-off patch to another subsystem
> > > > (what else?).
> > > >
> > > > I don't see where we are getting if we accept get_maintainer.pl
> > > > produces bad results and needs additional fixing in every system out
> > > > there (dozens) and when used by humans. All systems would need the
> > > > same filtering/checking rules and they need to keep in sync. What a
> > > > kernel developer would even need to do to fix something (add/remove
> > > > themselves)? Go and talk to a large unknown set of systems that
> > > > duplicate the same additional rules?
> > > >
> > > > And the only way to surface actual issues with get_maintainer.pl is to
> > > > start using it. In fact it's already widely used as is, so I am not
> > > > sure it's particularly bad.
> > >
> > > I'm not saying that get_maintainer.pl produces bad result. Depending
> > > on what is done, it could produce a very large output.
> > >
> > > Let's suppose that someone do something like globally renaming a
> > > widely-used kAPI, e. g. something like:
> > >
> > >         $ git ls-files|xargs sed s,mutex_,new_mutex_, -i
> > >
> > > A change like that would touch lots of subsystems, making get_maintainer.pl
> > > to spend a lot of time processing it, and producing thousands of
> > > entries (btw, we had a change somewhat similar to the above a long time
> > > ago when mutex API was introduced and most of the semaphores were converted
> > > to use mutex kAPI instead).
> >
> > What I end up doing in those cases is only Cc'ing the subsystem
> > maintainers. But that's a manual step of dropping all the driver and
> > SoC maintainers.
>
> Yeah, surely it would be a lot better if the maintainer's file would
> have a way to distinguish between driver and subsystem maintainers.
>
> > A related problem is if you want to put who should
> > apply the patch on To. That's maybe as simple as whether the
> > maintainer entry has a git tree.
>
> It is not that simple. Driver maintainers usually also point to the
> subsystem's tree.

I somewhat suspected that. Perhaps it should be explicit then.

Or remove those as it's redundant information.

> One way would be to look at the committer for the file.
>
> Another one would be to check if there are multiple maintainers
> for the same file, with different entries. If so, the first
> entry is the driver maintainer, and the last one, the subsystem
> maintainer. Yet, this is heuristics, and may lead to errors.

That's every binding file. :( There's me, the subsystem maintainer and
the file/device maintainer. Could be a 4th one if there's file
wildcard matching.

Rob



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux