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

[Thomas Monjalon <thomas@monjalon.net>]

04/11/2021 17:48, Ali Alnubani:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 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?

These considerations don't contradict with the option name.
If we have the option --skip-delegated (or --only-non-delegated),
we can always use this option in the automatic run.

But for a manual run, I expect the script to delegate a patch
the user explicitly wants to set, without requiring a "force" option.