-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix and update the pull request merge procedure #97
Fix and update the pull request merge procedure #97
Conversation
…on with PNJ and GDR. Still draft, but ready for comment.
…of what comes out Git Fusion from Perforce, and is closer to our migration target.
…iately clear to PNJ.
…uld be in the readership.
… also discards information about who did this procedure, which we don't want.
…exisitng licenses unless expressly stated otherwise in the contribution request.
…n of procedure steps.
…t doesn't happen automatically.
…ossibility of insisting on a clean trial merge before even starting this procedure.
…occur in context.
@thejayps merged #93 using this procedure (with me supervising) and yet today we were surprised that some branches were included that @thejayps didn't remember reviewing. That's because when he ran the checklist, he remembered reviewing "it" but "it" was in fact, quite a lot older than what he was merging. So there's a flaw in the checklist, because it just asks whether a review exists, not what it applied to. Fixed in 99ff0d0 |
The procedure doesn't deal with the case where the response to an issue is to merge the branch more than once: to master (for the long term) and to one or more version branches (to fix their defects). GitHub's pull request concept may not deal with this well either, given that Git itself doesn't seem to. |
…cedure in response to mini-review by @thejayps and @UNAA008 <#97 (comment)>.
Executing proc.merge.pull-request.
And it's a good job since I found an error in the command in this case. Fixed in e36c8f9 . Then undid and repeated the procedure. I didn't wait for CI results on https://github.com/Ravenbrook/mps/tree/merge/2023-01-31/pull-request-merge-procedure because I know there are no code changes, and there's no CI checker for reStructuredText yet (see #112 ). Merge procedure took 22 minutes. |
This is a branch to deal with #96 by creating a procedure for merging pull requests, but also to work on general git migration relating to merges.
You might wonder at the amount of effort that has gone in to this piece of bureaucracy. This is based on long experience with the MPS in defect prevention. This procedure (and others like it) are the final gatekeeper on the quality of the source code, and have been key to prevention in the past. GC bugs are very very nasty, especially when they happen to end-users of your user, who is likely a compiler developer, in a way that can't be seen or reproduced, let alone diagnosed and fixed.