spin icon indicating copy to clipboard operation
spin copied to clipboard

Non-maintainers can't merge their own approved PRs

Open calebschoepp opened this issue 1 year ago • 8 comments

With the recent governance change non-maintainers have lost the ability to merge their own PRs that have been approved. I've found this to be unergonomic because now I have to revert to pinging people in comments to get them to merge my PR.

calebschoepp avatar Sep 05 '24 15:09 calebschoepp

@calebschoepp would enabling auto-merge for approved PRs resolve this issue?

michelleN avatar Sep 05 '24 15:09 michelleN

fwiw I don't trust auto-merge right now. I just had it merge a PR that ultimately failed CI... 😬

lann avatar Sep 05 '24 15:09 lann

Who enables auto-merge? Me the non-maintainer or a maintainer who approved the PR?

calebschoepp avatar Sep 05 '24 15:09 calebschoepp

@lann We added some checks to the workflow to improve parity with main CI, but they are not 'required' (because they were added after the 'required' stuff got set up). So yeah auto-merge will pull the trigger once the original subset of checks passes, and not wait for the full checks. If you're an admin, you might be able to make the newer checks 'required' - I didn't have permissions.

</derailing>

itowlson avatar Sep 05 '24 21:09 itowlson

FYI: we were previously missing some "required" checks on certain CI jobs that should have blocked auto-merge on failure but did not. I updated these early this week to block on failure so we should be good to use auto-merging IMO.

rylev avatar Sep 06 '24 10:09 rylev

To your question @calebschoepp about the auto-merge process: now that all the checks are in place, I think the maintainer should hit "enable automerge" once they have lgtm'ed the PR.

michelleN avatar Sep 16 '24 20:09 michelleN

This seems like a reasonable conclusion for this ticket.

calebschoepp avatar Sep 16 '24 20:09 calebschoepp

Let's document this as a maintainer responsibility in CONTRIBUTING.md

michelleN avatar Sep 23 '24 13:09 michelleN