Re: [dpdk-ci] [PATCH v2 05/10] tools: add functionality for setting pw delegates

[Ali Alnubani <alialnu@oss.nvidia.com>]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, October 26, 2021 5:08 PM
> To: Ali Alnubani <alialnu@nvidia.com>
> Cc: ci@dpdk.org; jerinj@marvell.com; ferruh.yigit@intel.com;
> david.marchand@redhat.com; juraj.linkes@pantheon.tech
> Subject: Re: [PATCH v2 05/10] tools: add functionality for setting pw
> delegates
> 
> 18/10/2021 09:48, Ali Alnubani:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 21/09/2021 16:35, alialnu@nvidia.com:
> > > > From: Ali Alnubani <alialnu@nvidia.com>
> > > >
> > > > A new command was added to set patch delegates in Patchwork
> > > > based on the emails found in DPDK's MAINTAINERS file.
> > > >
> > > > Example usage:
> > > >   $ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
> > > >   $ ./pw_maintainers_cli.py --type series set_pw_delegate SERIES_ID
> > > >
> > > > Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> > > > ---
> > > > +    def set_delegate(self, patch_list, delegate):
> > > > +        """Set the delegate for a patch.
> > > > +        Only tries to set a delegate for patches that don't have
> > > > +        one set already.
> > >
> > > I'm not sure we should skip patches which are already delegated.
> > > If we use the command to explicitly delegate the patch,
> > > we should do it, right?
> > >
> > > The skip logic may be implemented at a higher level in the CI.
> >
> > I added an arg (--force_set_delegate) to force overriding delegates in v3.
> 
> Given the command is to set delegate, the force looks strange to me.
> Wouldn't it be more logical to add an option to skip already delegated
> patches?

Isn't it safer not to force overriding the delegate by default?
Users can send patches with the header "X-Patchwork-Delegate" to set a specific delegate. Maintainers might want to set another user as the delegate when there are multiple delegates in the MAINTAINERS file for a patch. Should we override these by default?