Re: Github PR label automation
Correct, it has no way to know if something has been put into ready to merge deliberately despite it failing checks etc so it won't mess with removing the label. Mark On Wed, Feb 12, 2020 at 10:39 AM Dr. Matthias St. Pierre wrote: > > > check. It will not move to 'ready to merge' state automatically > > unless (or until) all CI passes. (I'll do a PR for the tool with that > > change shortly). > > If the does not automatically remove the "ready to merge" label, but only > refrains from setting it automatically, that's a good compromise I can live > with. > > Matthias >
Re: Github PR label automation
Okay, I'll leave things as they are with those issues and just add the addition of notification on urgent labels. As to the earlier question form the thread as why this isn't a github automation: the actual reason I was writing the script was to get some on-demand statistics for my own interest and it just turned out this was something that bugged me that I could use the same framework for (I did a trivial review and thought how annoying it would be to have to manually set a reminder myself to do a future action). I was a pretty trivial (few hours) script, so I have no concern if it is done differently or not used in the future should a better way present itself. Mark On Sun, Feb 9, 2020 at 12:19 AM Matt Caswell wrote: > > > > On 08/02/2020 15:56, Mark J Cox wrote: > > I've currently got a cron job running every hour that looks at open PR > > requests against github openssl repo and does various actions. So if > > you were wondering why I was altering labels and making comments at > > 4am, now you know. No doubt we'll use some tool user for this in the > > future. > > > > So right now here's what it does: > > > > Every hour it looks at open PRs that are labelled "approval: done". > > If 24 hours has elapsed since that label was assigned and if there > > have been no comments made to the PR since the label was assigned then > > it is automatically moved to "approval: ready to merge" with a comment > > added to trigger notifications. So if you want to stop something > > going to "ready to merge" just add any comment to the PR. > > > > I'm thinking of using this script also to 1) collect interesting stats > > and 2) do some other actions. So if there's some automation you'd > > like to see just add an enhancement issue against the openssl/tools > > repo. > > > > 1 Matt already asked for committer notification trigger for anything > > labelled Urgent. > > > > 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. So I'll likely add that next week (if > > "approval: done" label and has comments since that label then remove > > the label and add a comment 'please review if this is really approval: > > done'. If the approval: done label gets set again then after 24 hours > > the existing automation will trigger. #10786 is a good example of > > this. > > I'm less sure about this. I think there are probably many examples where > this is not the case. E.g. a quick review found these recent cases: > > https://github.com/openssl/openssl/pull/11003 > > https://github.com/openssl/openssl/pull/11015 > > https://github.com/openssl/openssl/pull/10992 > > https://github.com/openssl/openssl/pull/10991 > > https://github.com/openssl/openssl/pull/10990 > > https://github.com/openssl/openssl/pull/10989 > > I would be more minded to think that if a comment invalidates an > "approval done" then the committer should explicitly decide to do that. > > Matt
Re: Github PR label automation
Please don’t automatically drop the "appoval: done" label after a comment. I feel that is not uncommon for comments to be added that in no way invalidate the approval. I agree with not switching to “ready to merge” if there are comments — manual intervention in this case is required to judge the relevancy. Agreed also over the “urgent” label. Pauli -- Dr Paul Dale | Distinguished Architect | Cryptographic Foundations Phone +61 7 3031 7217 Oracle Australia > On 9 Feb 2020, at 1:56 am, Mark J Cox wrote: > > I've currently got a cron job running every hour that looks at open PR > requests against github openssl repo and does various actions. So if > you were wondering why I was altering labels and making comments at > 4am, now you know. No doubt we'll use some tool user for this in the > future. > > So right now here's what it does: > > Every hour it looks at open PRs that are labelled "approval: done". > If 24 hours has elapsed since that label was assigned and if there > have been no comments made to the PR since the label was assigned then > it is automatically moved to "approval: ready to merge" with a comment > added to trigger notifications. So if you want to stop something > going to "ready to merge" just add any comment to the PR. > > I'm thinking of using this script also to 1) collect interesting stats > and 2) do some other actions. So if there's some automation you'd > like to see just add an enhancement issue against the openssl/tools > repo. > > 1 Matt already asked for committer notification trigger for anything > labelled Urgent. > > 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. So I'll likely add that next week (if > "approval: done" label and has comments since that label then remove > the label and add a comment 'please review if this is really approval: > done'. If the approval: done label gets set again then after 24 hours > the existing automation will trigger. #10786 is a good example of > this. > > Mark
Re: Github PR label automation
Thanks Dmitry; I hope that the comment triggers notifications to the creator without mentioning them? (let me know if you get something changed labels that doesn't) Mark On Sat, Feb 8, 2020 at 4:57 PM Dmitry Belyavsky wrote: > > Dear Mark, > > Thank you for a nice job! > > As the reviewers are expected to commit the PRs, could you also add the > reviewers' names as a part of the notification? > > On Sat, Feb 8, 2020 at 6:56 PM Mark J Cox wrote: >> >> I've currently got a cron job running every hour that looks at open PR >> requests against github openssl repo and does various actions. So if >> you were wondering why I was altering labels and making comments at >> 4am, now you know. No doubt we'll use some tool user for this in the >> future. >> >> So right now here's what it does: >> >> Every hour it looks at open PRs that are labelled "approval: done". >> If 24 hours has elapsed since that label was assigned and if there >> have been no comments made to the PR since the label was assigned then >> it is automatically moved to "approval: ready to merge" with a comment >> added to trigger notifications. So if you want to stop something >> going to "ready to merge" just add any comment to the PR. >> >> I'm thinking of using this script also to 1) collect interesting stats >> and 2) do some other actions. So if there's some automation you'd >> like to see just add an enhancement issue against the openssl/tools >> repo. >> >> 1 Matt already asked for committer notification trigger for anything >> labelled Urgent. >> >> 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. So I'll likely add that next week (if >> "approval: done" label and has comments since that label then remove >> the label and add a comment 'please review if this is really approval: >> done'. If the approval: done label gets set again then after 24 hours >> the existing automation will trigger. #10786 is a good example of >> this. >> >> Mark > > > > -- > SY, Dmitry Belyavsky
Re: Github PR label automation
Dear Mark, Thank you for a nice job! As the reviewers are expected to commit the PRs, could you also add the reviewers' names as a part of the notification? On Sat, Feb 8, 2020 at 6:56 PM Mark J Cox wrote: > I've currently got a cron job running every hour that looks at open PR > requests against github openssl repo and does various actions. So if > you were wondering why I was altering labels and making comments at > 4am, now you know. No doubt we'll use some tool user for this in the > future. > > So right now here's what it does: > > Every hour it looks at open PRs that are labelled "approval: done". > If 24 hours has elapsed since that label was assigned and if there > have been no comments made to the PR since the label was assigned then > it is automatically moved to "approval: ready to merge" with a comment > added to trigger notifications. So if you want to stop something > going to "ready to merge" just add any comment to the PR. > > I'm thinking of using this script also to 1) collect interesting stats > and 2) do some other actions. So if there's some automation you'd > like to see just add an enhancement issue against the openssl/tools > repo. > > 1 Matt already asked for committer notification trigger for anything > labelled Urgent. > > 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. So I'll likely add that next week (if > "approval: done" label and has comments since that label then remove > the label and add a comment 'please review if this is really approval: > done'. If the approval: done label gets set again then after 24 hours > the existing automation will trigger. #10786 is a good example of > this. > > Mark > -- SY, Dmitry Belyavsky
