[v2,0/1] Add add-maintainer.py script

Message ID cover.1691049436.git.quic_gurus@quicinc.com
Headers
Series Add add-maintainer.py script |

Message

Guru Das Srinagesh Aug. 3, 2023, 8:23 a.m. UTC
  When pushing patches to upstream, the `get_maintainer.pl` script is used to
determine whom to send the patches to. Instead of having to manually process
the output of the script, add a wrapper script to do that for you.

The add-maintainer.py script adds maintainers (and mailing lists) to a patch,
editing it in-place.

Thanks to Bjorn for being a sounding board to this idea and for his valuable
suggestions.

Please try out this script with `--verbosity debug` for verifying that it's
doing "the right thing". I've tested this with a patch series from various
subsystems to ensure variety of maintainers and lists output and found it to be
doing what it is supposed to do.

I referred to the following links during development of this script:
- https://stackoverflow.com/questions/4427542/how-to-do-sed-like-text-replace-with-python
- https://stackoverflow.com/questions/4146009/python-get-list-indexes-using-regular-expression
- https://stackoverflow.com/questions/10507230/insert-line-at-middle-of-file-with-python

v1 -> v2:
- Added set-union logic based on Pavan's comments [1] and Bjorn's early suggestion
- Expanded audience and added more mailing lists to get more review comments and feedback

[1] https://lore.kernel.org/lkml/63764b84-3ebd-4081-836f-4863af196228@quicinc.com/

Guru Das Srinagesh (1):
  scripts: Add add-maintainer.py

 scripts/add-maintainer.py | 113 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 113 insertions(+)
 create mode 100755 scripts/add-maintainer.py
  

Comments

Neil Armstrong Aug. 3, 2023, 9:16 a.m. UTC | #1
Hi,

On 03/08/2023 10:23, Guru Das Srinagesh wrote:
> When pushing patches to upstream, the `get_maintainer.pl` script is used to
> determine whom to send the patches to. Instead of having to manually process
> the output of the script, add a wrapper script to do that for you.
> 
> The add-maintainer.py script adds maintainers (and mailing lists) to a patch,
> editing it in-place.

FYI the b4 prep command does this:
https://github.com/mricon/b4/blob/e8045d1353165cc065b2f1b180bf1b0846af510e/b4/ez.py#L2055

Perhaps it could be useful to make sure the output is similar ?

So far I've been very satisfied by the output of b4 auto_to_cc.

Thanks,
Neil

> 
> Thanks to Bjorn for being a sounding board to this idea and for his valuable
> suggestions.
> 
> Please try out this script with `--verbosity debug` for verifying that it's
> doing "the right thing". I've tested this with a patch series from various
> subsystems to ensure variety of maintainers and lists output and found it to be
> doing what it is supposed to do.
> 
> I referred to the following links during development of this script:
> - https://stackoverflow.com/questions/4427542/how-to-do-sed-like-text-replace-with-python
> - https://stackoverflow.com/questions/4146009/python-get-list-indexes-using-regular-expression
> - https://stackoverflow.com/questions/10507230/insert-line-at-middle-of-file-with-python
> 
> v1 -> v2:
> - Added set-union logic based on Pavan's comments [1] and Bjorn's early suggestion
> - Expanded audience and added more mailing lists to get more review comments and feedback
> 
> [1] https://lore.kernel.org/lkml/63764b84-3ebd-4081-836f-4863af196228@quicinc.com/
> 
> Guru Das Srinagesh (1):
>    scripts: Add add-maintainer.py
> 
>   scripts/add-maintainer.py | 113 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 113 insertions(+)
>   create mode 100755 scripts/add-maintainer.py
>
  
Guru Das Srinagesh Aug. 10, 2023, 6:49 p.m. UTC | #2
On Aug 03 2023 11:16, Neil Armstrong wrote:
> Hi,
> 
> On 03/08/2023 10:23, Guru Das Srinagesh wrote:
> >When pushing patches to upstream, the `get_maintainer.pl` script is used to
> >determine whom to send the patches to. Instead of having to manually process
> >the output of the script, add a wrapper script to do that for you.
> >
> >The add-maintainer.py script adds maintainers (and mailing lists) to a patch,
> >editing it in-place.
> 
> FYI the b4 prep command does this:
> https://github.com/mricon/b4/blob/e8045d1353165cc065b2f1b180bf1b0846af510e/b4/ez.py#L2055
> 
> Perhaps it could be useful to make sure the output is similar ?
> 
> So far I've been very satisfied by the output of b4 auto_to_cc.

Thank you - let me check this tool out.

Guru Das.
  
Guru Das Srinagesh Aug. 10, 2023, 6:55 p.m. UTC | #3
On Aug 03 2023 01:23, Guru Das Srinagesh wrote:
> When pushing patches to upstream, the `get_maintainer.pl` script is used to
> determine whom to send the patches to. Instead of having to manually process
> the output of the script, add a wrapper script to do that for you.
> 
> The add-maintainer.py script adds maintainers (and mailing lists) to a patch,
> editing it in-place.

Could I request reviews from the other maintainers as well, please? Just to see
if I should continue working on this script or if the `b4` tool obviates the
need for such a script.

Thank you.

Guru Das.
  
Guru Das Srinagesh Aug. 16, 2023, 5:15 p.m. UTC | #4
Thanks for the comments, Krzysztof.

On Aug 15 2023 23:06, Krzysztof Kozlowski wrote:
> On 10/08/2023 20:55, Guru Das Srinagesh wrote:
> > On Aug 03 2023 01:23, Guru Das Srinagesh wrote:
> >> When pushing patches to upstream, the `get_maintainer.pl` script is used to
> >> determine whom to send the patches to. Instead of having to manually process
> >> the output of the script, add a wrapper script to do that for you.
> >>
> >> The add-maintainer.py script adds maintainers (and mailing lists) to a patch,
> >> editing it in-place.
> > 
> > Could I request reviews from the other maintainers as well, please? Just to see
> > if I should continue working on this script or if the `b4` tool obviates the
> > need for such a script.
> 
> I send a bit of patches but I use very simple workflow. It is really
> simple, so simple, that I was always surprised how people can make their
> life difficult with some complicated process to send patches... and then
> obviously skip some maintainers, because of that process.

Exactly - this script aims to solve precisely that problem. It fills the gap
between running `get_maintainers.pl` and having to manually edit its output to
add "To: " and "Cc: " and somehow incorporate it in the body of the patch(es).

With this script, the workflow would be as simple as:

  1. Generate patches using `git format-patch`
  2. Run `add-maintainer.py` on the above patches
  3. `git send-email` the patches.

That's it - no need to manually work with email addresses.
  
> I almost always feed git send-email with addresses from
> scripts/get_maintainers.pl. This tool would not bring any benefits to my
> simple workflow.

In the light of the 3-step workflow I've envisioned above, could you please
elaborate why not? If anything, it will only save a developer's time.

> For newcomers, OTOH, I would either recommend simple workflow or just
> use b4. Why? Because if you cannot use git-send-email, then it means
> your email setup will make your life difficult and adding maintainers to
> existing patch won't help you.

You've mentioned a "simple workflow" many times - could you please share more
details on the steps you follow in your workflow for sending patches?

> This tool depends on the command line and shell interface of
> scripts/get_maintainers.pl which is another reason why it might not be a
> good idea.

Could you please elaborate on why depending on the output of
`get_maintainer.pl` is a bad idea? It's what everyone uses, no?

Thank you.

Guru Das.
  
Guru Das Srinagesh Aug. 19, 2023, 1:48 a.m. UTC | #5
On Aug 18 2023 10:33, Neil Armstrong wrote:
> My opinion is that it would be a better idea to add a new output mode
> to scripts/get_maintainer.pl than adding another script on top of it.

Sorry, I don't follow. The problem that this script is solving is getting the
output of `get_maintainer.pl` neatly into a patch according to this scheme:

  1. Generate patches using `git format-patch`
  2. Run `add-maintainer.py` on the above patches
  3. `git send-email` the patches.

Not sure how adding a new output mode to `get_maintainer.pl` fits in this
problem space.

Unless you mean to add a switch to it so that it automatically
edits the patch in-place (like `add-maintainer.py` does) and adds all the
addresses directly to the patch - in which case, that would be feature creep.

> Or document somewhere how to use get_maintainer.pl with git-format-patch
> without any additional scripts.

IMHO, `Documentation/process/submitting-patches.rst` should be updated with at
least one or two options addressing how to get from the aforementioned
step #1 -> step #3 addressing the problem that is being solved by step #2. In
this vacuum, every developer and maintainer appears to have their own solution
that works for them.

Thank you.

Guru Das.