justinmclean commented on PR #47:
URL:
https://github.com/apache/comdev-working-groups/pull/47#issuecomment-5008404007
Thanks Rich, this is a great start. Most of my comments are on the two
Python scripts, plus a few spots in the docs that don't quite line up.
On validate-application.py:
The blank check doesn't actually work for a bunch of the required fields. It
finds the label and then reads to the end of the line, so when the label in the
list isn't the full thing with its colon, e.g. Conference location, Traveling
from, Amount requested, Venue, reviewers will be present, PMC APPROVAL, and
importantly TOTAL REQUESTED, it just matches partway through and treats the
rest of the label as the value. So an empty TOTAL REQUESTED (max $400): $ line
comes back as "filled". In other words, some of the fields never really get
checked.
On update-summary.py:
If a category's SVN folder doesn't exist yet, it gets skipped, but the
grand-totals step still reads totals for all five categories — so the first
time we set up a new FY with only one or two categories active, it'll either
KeyError and fall over, or quietly use stale numbers. I'd just initialise each
category's totals to zero before the folder check.
Second, approved_count includes everything that isn't denied or withdrawn,
including APPLIED. If we're dropping application records in those folders
before a decision, the approved totals will be too high. If files only get
created on approval, then it's fine - but the status list has APPLIED in it, so
it's a bit ambiguous.
Minor one: the amount parser strips all non-digits from the line, so a stray
number in a note like $3,000 (2 people) would become 30002.
And the per-record Status: line lists a different set of statuses than the
master list in svn-data-format.md (that one adds RECEIPTS_FILED and WITHDRAWN).
On the docs:
We say a PMC vote is needed for anything over $2,000, but the per-category
rules only call for it on summits. Travel grants go up to $3,000, so most of
the bigger ones would cross $2,000 too. Can we spell out whether those need a
PMC vote? Right now the ">$2,000" rule and the "two evaluators agree =
approved" travel path read as contradictory.
Also meetups are capped at $400 but the single-approver path is written for
"under $300" - nothing says what happens between $300 and $400.
Last one, on SKILL.md: using Google Flights, Skyscanner, Booking.com etc. to
sanity-check costs is fine as a manual step, but I'd be careful about anything
automated hitting those sites, and about an AI just making up a number. The
"must be reproducible from public sources so the applicant can verify it" line
is the right safeguard. I'd make that a hard requirement rather than a
nice-to-have.
And there's nothing about how long we keep the private financial records or
when we delete them, which we should probably address.
Happy to help with any of the script fixes if useful. I'll also note that
unit tests would have likely caught the script issues above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
