-
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 26, 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 in the specific 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 intoto themasterbranch itself).