Add a short maintainer guide for merge:batch and link it from the existing merge policy docs. Lock in the source-validation CI fixes discovered during the merge-batch end-to-end exercise so shallow checkout and missing base-branch fetch regressions fail the workflow contract test.
5.7 KiB
5.7 KiB
Merging Pull Requests
Policy: we always Merge PRs on GitHub so contributors get credit. We never Close a PR after integrating their work locally.
Always merge via GitHub
- Use the GitHub UI "Squash and merge" for every accepted PR.
- The PR must show as Merged, not Closed. That way the contributor appears in the repo’s contribution graph and the PR is clearly linked to the merge commit.
- Do not integrate a PR by squashing locally, pushing to
main, and then closing the PR. That would show "Closed" and the contributor would not get proper credit. - Before merging, require the normal PR checks from
.github/workflows/ci.ymlto be green. If the PR touchesSKILL.md, also require the separateskill-reviewworkflow to pass. - For PRs that touch
SKILL.mdor risky guidance, require a real manual logic review in addition to the automated checks. Confirm the instructions, failure modes, andrisk:label make sense before merging. - For ordered multi-PR maintainer batches, use Merge Batch as the operational shortcut and keep this document as the policy reference.
If the PR has merge conflicts
Resolve conflicts on the PR branch so the PR becomes mergeable, then use "Squash and merge" on GitHub.
Generated files policy
- Treat
CATALOG.md,skills_index.json, anddata/*.jsonas derived artifacts, not contributor-owned source files. README.mdis mixed ownership: contributor prose edits are allowed, but workflow-managed metadata is canonicalized onmain.- If derived files appear in a PR refresh or merge conflict, prefer
main's side and remove them from the PR branch instead of hand-maintaining them there. - Do not block a PR only because shared generated files would be regenerated differently after other merges.
mainauto-syncs the final state after merge. - If a skill PR leaves
risk: unknown, that is not automatically a blocker. Maintainers can review the suggested classification withnpm run audit:skills, optionally runnpm run sync:risk-labelslocally after merge, and still keep the contributor PR source-only.
Steps (maintainer resolves conflicts on the contributor’s branch)
- Fetch the PR branch
git fetch origin pull/<PR_NUMBER>/head:pr-<PR_NUMBER> - Checkout that branch
git checkout pr-<PR_NUMBER> - Merge
maininto it
git merge origin/main
Resolve any conflicts in the working tree. For generated registry files (CATALOG.md,data/*.json,skills_index.json), prefermain's version and remove them from the contributor branch:git checkout --theirs CATALOG.md data/catalog.json skills_index.jsonIfREADME.mdconflicts only because of workflow-managed metadata, prefermain's side there too. Keep contributor prose edits when they are real source changes. - Commit the merge
git add .thengit commit -m "chore: merge main to resolve conflicts"(or leave the default merge message). - Push to the same branch the PR is from
If the PR is from the contributor’s fork branch (e.g.sraphaz:feat/uncle-bob-craft), you need push access to that branch. Options:- Preferred: Ask the contributor to merge
maininto their branch, fix conflicts, and push; then you use "Squash and merge" on GitHub. - If you have a way to push to their branch (e.g. they gave you permission, or the branch is in this repo), push:
git push origin pr-<PR_NUMBER>:feat/uncle-bob-craft(replace with the actual branch name from the PR).
- Preferred: Ask the contributor to merge
- On GitHub: The PR should now be mergeable. Click "Squash and merge". The PR will show as Merged.
If the contributor resolves conflicts
Ask them to:
git checkout <their-branch>
git fetch origin main
git merge origin/main
# resolve conflicts, then drop derived files from the PR if they appear:
# CATALOG.md, skills_index.json, data/*.json
git add .
git commit -m "chore: merge main to resolve conflicts"
git push origin <their-branch>
Then you use "Squash and merge" on GitHub. The PR will be Merged, not Closed.
Rare exception: local squash (avoid if possible)
Only if merging via GitHub is not possible (e.g. contributor unreachable and you must integrate their work, or a one-off batch), you may squash locally and push to main. In that case:
- Add a Co-authored-by line to the squash commit so the contributor is still credited (see GitHub: Creating a commit with multiple authors).
- Close the PR with a comment explaining why it was integrated locally and that attribution is in the commit.
- Prefer to avoid this pattern in the future so PRs can be Merged normally.
Summary
| Goal | Action |
|---|---|
| Give contributors credit | Always use Squash and merge on GitHub so the PR shows Merged. |
| PR has conflicts | Resolve on the PR branch (you or the contributor), then Squash and merge. |
| Never | Integrate locally and then Close the PR without merging. |