develooper Front page | perl.perl5.porters | Postings from August 2022

Unifying our AUTHORS tools: replacing checkAUTHORS.pl withupdateAUTHORS.pl

From:
demerphq
Date:
August 8, 2022 20:22
Subject:
Unifying our AUTHORS tools: replacing checkAUTHORS.pl withupdateAUTHORS.pl
Message ID:
CANgJU+XfHS+vENGV2NP+rP588xJ7E1hqFp03DUkoYn3=-wCxnA@mail.gmail.com
Hi Folks,

I pushed a PR today to unify checkAUTHORS.pl featureset with
updateAUTHORS.pl and to remove checkAUTHORS.pl entirely. This was a
consequence of our infra being unable to deal with "dont ever list
this person" without a lot of bodge (eg, double entry between
checkAUTHORS.pl and updateAUTHORS.pl).
https://github.com/Perl/perl5/pull/20070

updateAUTHORS.pl uses an easier to process and extend dataset,
understands the impact of .mailmap, and can handle hashed contributor
exclusions. checkAUTHORS.pl contained a bunch of data which I think
was out of date, somewhat cryptic and not easy to maintain. When I
created updateAUTHORS.pl I essentially merged all the data it
contained into AUTHORS or .mailmap, (beyond out of date data on who
are committers), so I dont think removing it should matter.

The PR has a bunch of details on the changes I have made.

The main reason I am writing here is to call attention to this change
and especially the fact that checkAUTHORS.pl wont exist anymore. I
thought about making checkAUTHORS.pl a wrapper around updateAUTHORS.pl
but I have a feeling the tool has a very small user base and there is
no real need. If I am wrong then I am happy to add it.

Another reason I am writing here is to mention two changes that could
need rectification before we merge.

1. t/porting/authors.t now uses updateAUTHORS.pl to do its thing. In
order to simplify the script I removed the logic about making it scan
only recent changes. I do so also because I saw some missing entries
and I believe it is at least plausible from other experiences that
sometimes the data passed into the script would make it skip commits
under certain circumstances. IN theory this shouldnt matter too much.
It doesn't take that long to read all our commit data.  If it does
then we can add that bck.

2. t/porting/pending-authors.pl - i dont understand why this script
was needed. t/porting/authors.t will validate that the .mailmap and
AUTHORS file are correct before people push when they run tests
locally, and it will validate that they are correct in our CI. So I
dont understand what this test was testing. There is a chicken and egg
in that if a contributor runs make test before committing they might
push a PR that would then fail the test after they commit and push. A
commit bit holder could push directly and bypass CI, and that might
introduce an inconsistent state, but the likelyhood would be low that
someone would have a commit bit and also not be listed in AUTHORS
already. Basically the committer would have to be playng silly buggers
with their author details and then push directly bypassing CI at the
same time. If that happens we can deal with by giving them a good
scolding for being irresponsible.  The point is that with the latest
version of the pipeline AUTHORS and .mailmap should always be correct,
in which case this test seems to be redundant. Please correct me if I
misunderstand how the pieces fit together here.

(Note there are other changes listed in the PR, but the above are the
two which i think are the most likely to need correction before
merge.)

So please let me know if you have a bunch of tooling or scripts that
use checkAUTHORS.pl, if there is a need im sure we can come up with a
back compat shim.

The patch is marked as DRAFT for now so folks know I'm planning on
pushing fixes before it gets merged. I figured there were likely to be
change or enhancement requests, and I would like to get the feedback
early. I will likely push a few additional report options before the
patch settles down, but the general spirit of what I think we should
do is ready for review.

Also, I have plans to respond to some of the other feedback to
updateAUTHORS.pl, especially relating to C<.mailmap> containing
"redundant" entries. They aren't redundant from the POV of the program
flow, but I expect I can rework things so they can be made redundant
and removed. I do not know if I want to do this as part of this PR,
depending on my time availability I guess, but I do know that I would
prefer to do it on top of this merge. Having two tools with different
ideas of how AUTHORS should look is a bit less than ideal.

thanks for your time,
yves
-- 
perl -Mre=debug -e "/just|another|perl|hacker/"



nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About