Github PR label automation
Dr. Matthias St. Pierre
Matthias.St.Pierre at ncp-e.com
Sat Feb 8 20:33:28 UTC 2020
First of all, thank you Mark for implementing the notification daemon. You did a great job
and I think it's very useful. Here are some comments and thoughts about your last mail.
> No doubt we'll use some tool user for this in the future.
Can't you just use an API-token generated for the openssl-machine account,
as the issue closing bot does?
A propos bot: I thought that GitHub provides official support for this kind of watch-bots
we are seeking and that they can be configured or programmed in an event driven fashion.
Executing specific actions (like setting labels or posting messages) in response to specific
GitHub events (approval added, change requested, timer expired, etc.) would have some
advantages compared to an external timer based approach. Did someone of you consider
this option and do some investigations in that direction? Please don't misinterpret my
question, I think that the current cron job solution is already a great improvement and
a big step forward.
> 2 If there were comments made after "approval: done" then I think we
> really ought to drop the "approval: done" label as the comments likely
> invalidated the approval. ...
I don't think that an *arbitrary comment* should automatically reset the approval state.
Anybody could just post a "nice job" comment or some side note. Resetting the approval
state could happen automatically if a *team member* posts a review with *change requests*.
But not in any other case (e.g. a change requests by a non-team member or an additional
Also, I am strongly convinced that making the transition from the [approval: * review pending]
to the [approval:done] state should remain a *purely manual* state. I don't think it's a good idea
to leave this decision to an "artificial intelligence" whatsoever. And just counting whether the
number of approvals has reached the required minimum is too simplistic anyway.
Just imagine the pull request has reached the required number of reviews, but the
submitter or a reviewer is still waiting for another important outstanding review.
Do you really want the bot to interfere?
What about the existing approvals? They need to be dismissed if 'too much' has changed,
but not if the author pushed a trivial fixup addressing an approving review. Do you really
want to leave this decision to a bot?
This approach will just not work.
It is really almost no extra effort if we demand that the second reviewer sets the
[approval: done] label manually, unless he sees that there are still unresolved discussions
in progress. Only the grace period handling should be automated IMHO.
More information about the openssl-project