- 
                Notifications
    You must be signed in to change notification settings 
- Fork 71
Checklist for testing and merging a PR
        Erik Martin-Dorel edited this page Sep 15, 2021 
        ·
        21 revisions
      
    - install the git-prandgit-prwextra Git commands;
- 
cdin thelearn-ocamlrepository (in the sequel, we assume the remoteoriginis bound to URL[email protected]:ocaml-sf/learn-ocaml.git);
- run git pr 200 origin -fto force-fetch a read-only branch for PR #200,
 orgit prw 200 origin -fto get a read-and-write branch, i.e.,git pushwill land in the PR-author's work branch.
- The CI checks must be green.
- Non-trivial PRs should have been tested locally (see the Guidelines above).
- Ideally, the commits messages should follow the Conventional Commits guidelines.
 This should ease the generation of Release-Notes using release-please.
- New user-facing features should be documented within the PR, in directory docs/.
There are 3 PR merging strategies in GitHub (Merge, Merge/Squash, Merge/Rebase).
But for merging PRs, we should rather use Merge or Merge/Squash:
- 
Mergeis always a good, standard choice;
- 
Merge/Squashis especially fine if we don't want to keep the commit history of the PR branch or if the PR already contains only one commit;
- 
whereas Merge/Rebasehas two drawbacks:- there is no mention anywhere of the PR number in the generated commits messages;
- and the resulting commits are not GPG-signed (cf. the missing Verifiedbadge on those commits).
 actually Merge/Rebasemight be handy only in the case of "nested PR merging" (namely: we wouldMerge/Rebasean intermediate PR into a feature branch (for which the intermediate PR number would be useless), then create a subsequent PR, and finallyMergeit from the feature branch to themasterbranch itself).