Prevent merging PRs which have CI checks run a week+ ago

[mkArtakMSFT]

No description provided.

[ghost]

Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at.

[wtgodbe]

LGTM - I asked other folks on the team for feedback, so let's wait a bit to see if anybody has other thoughts

[wtgodbe]

Actually, can we change the approve action to only approve if somebody else has already approved the PR? That way we don't accidentally merge any PRs that have only been reviewed by fabricbot. Or just remove the rejection without approving?

[mkArtakMSFT]

can we change the approve action to only approve if somebody else has already approved the PR?

Can't find anything relevant :(

[wtgodbe]

Hmm, maybe lock when adding the label and unlock when removing then?

EDIT - never mind, that locks the conversation. I have some concern about the bot approving PRs that may not have been approved by anyone else yet - maybe a malicious person could find that there's a way to get their PR approved without anybody actually looking at it. Maybe instead the bot adds the blocked label or the no-merge label at the same time it adds the rerun-CI label, and we remove the follow-up action? Then the dev could just remove both labels themselves. Curious to see what others think about this, though.

[HaoK]

Could we instead just have the bot do /azp run on old green non-draft PRs?

[dougbu]

Could we instead just have the bot do /azp run on old green non-draft PRs?

We discussed that option but realized aspnetcore alone has many PRs that would get an /azp run in them, which may have a significant resource impact. This way the runs happen only when the owner or one of the repo collaborators feels the PR is actually ready to go.

On the other hand, there's a related point about servicing PRs: Because those often sit while the branches are closed and very rarely hit breaking changes from other PRs, might be good to focus on 'main' w/ this new action.

[mkArtakMSFT]

Instead of approving the PR, maybe we should not do any of it, and instead just automate further by dropping an /azp run when the pending-ci-rerun label is removed?

[dougbu]

I suspect this is fine but have one concern…

[mkArtakMSFT]

@wtgodbe waiting for your feedback here.

[wtgodbe]

Works for me

[Nick-Stanton]

@mkArtakMSFT @wtgodbe @dougbu

Encountered this here

[ghost]

Hi @Nick-Stanton. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.