Skip to content

Commit b1ba8a9

Browse files
AlexanderLaninAlexanderLanin
committed
merge strategy
1 parent f89402a commit b1ba8a9

1 file changed

Lines changed: 264 additions & 0 deletions

File tree

‎docs/explanation/01-source-code-infrastructure.md‎

Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,270 @@ In S-CORE, policy intent is expressed centrally via the infrastructure-as-code t
5555

5656
**Biggest gap**: there is no common policy definition or enforcement strategy, and the current state of policy across repositories is not well documented or visible. Few repositories apply policies. GitHub Actions allowlisting is not yet implemented or documented as a security baseline.
5757

58+
### 1.3.1 PR Merge Strategy 🟢
59+
60+
S-CORE repositories use GitHub's **Squash and merge** method when merging pull
61+
requests into main. The other methods — **Create a merge commit** and **Rebase
62+
and merge** — are disabled. Exceptions exist.
63+
64+
The guiding principle: the pull request is the unit of review and integration.
65+
The resulting squash commit is the corresponding unit in the permanent main
66+
branch history.
67+
68+
:::{important} GitHub merge methods vs. local Git operations
69+
GitHub's three merge methods are distinct from similarly named local Git operations (`git merge`, `git rebase`, `git rebase -i`).
70+
71+
This policy covers only how an accepted pull request is recorded on main. It
72+
does not prescribe how many commits a contributor creates locally, whether they
73+
use merge or rebase during development, or whether fixup commits appear inside
74+
the PR branch.
75+
76+
After review has started, avoid rewriting published history unless coordinated
77+
with reviewers — it can invalidate commit-based review context.
78+
:::
79+
80+
#### The three strategies
81+
82+
| Method | Result on main | PR boundary marker | Intermediate commits land on main |
83+
|---|---|---|---|
84+
| Create a merge commit | All PR commits + merge commit | Yes (merge commit) ✅ | Yes ❌ |
85+
| **Squash and merge** | One new commit | Yes (squash commit) ✅ | No ✅ |
86+
| Rebase and merge | Each PR commit recreated | No ❌ | Yes ❌ |
87+
88+
##### Create a merge commit
89+
90+
GitHub creates a new commit on main with two parents: the tip of the PR branch
91+
and the previous tip of main. All PR commits land on main as-is, followed by
92+
this merge commit. The result is a non-linear history — `git log --graph` shows
93+
the branch forking off and rejoining.
94+
95+
A PR that synced with main during development (via `Merge branch 'main' into
96+
feature`) will also have those sync merge commits in the history.
97+
98+
*All these commits end up in main history*
99+
100+
:::{mermaid}
101+
%%{init: {'themeVariables': {'fontSize': '11px', 'git0': '#2171b5', 'git1': '#6baed6', 'gitInv0': '#fff', 'gitInv1': '#fff'}}}%%
102+
gitGraph
103+
commit id: "prev"
104+
branch feature
105+
checkout feature
106+
commit id: "implement"
107+
checkout main
108+
commit id: "other PR"
109+
checkout feature
110+
merge main id: "sync main"
111+
commit id: "fix review"
112+
commit id: "fix review 2"
113+
commit id: "nit"
114+
commit id: "address comment"
115+
checkout main
116+
merge feature id: "Merge #42"
117+
:::
118+
119+
After merging, main contains all PR commits plus the merge commit. The branch is
120+
visible as a fork in history.
121+
122+
**Pros**
123+
124+
- Preserves the full commit history exactly as developed, including per-author
125+
attribution.
126+
- The merge commit marks the PR boundary structurally — `git branch -d` detects
127+
the branch as merged, `git revert -m 1` reverts the entire PR as a unit, and
128+
`git bisect` can narrow to individual commits within the PR.
129+
- Safe for branches that continue to be used after merging.
130+
131+
**Cons**
132+
133+
- Merge commits from upstream syncs and work-in-progress "fix findings" commits
134+
both land on main, weakening `git log` unless authors curate commits before
135+
merging.
136+
- `git bisect` may land on intermediate commits that do not build or pass tests.
137+
- History becomes non-linear, which some tools and people find harder to follow.
138+
139+
##### Squash and merge
140+
141+
GitHub combines all PR commits into a single new commit and appends it to main
142+
as a fast-forward. No merge commit is added — main stays linear. The squashed
143+
commit has a new SHA with no structural link to the original PR branch. The
144+
original PR commits are not ancestors of main: they remain visible in the GitHub
145+
PR view but are not part of the permanent Git history and will not be present in
146+
clones once the branch is deleted.
147+
148+
:::{mermaid}
149+
%%{init: {'themeVariables': {'fontSize': '11px'}}}%%
150+
gitGraph
151+
commit id: "prev"
152+
branch feature
153+
checkout feature
154+
commit id: "implement"
155+
checkout main
156+
commit id: "other PR"
157+
checkout feature
158+
merge main id: "sync main"
159+
commit id: "fix review"
160+
commit id: "fix review 2"
161+
commit id: "nit"
162+
commit id: "address comment"
163+
checkout main
164+
commit id: "squash(#42)"
165+
:::
166+
167+
After merging, main contains one new commit per PR. Intermediate commits are
168+
excluded from main history.
169+
170+
**Pros**
171+
172+
- One commit per PR on main — clean, linear, easy to scan and bisect at PR
173+
granularity.
174+
- Work-in-progress and "fix findings" commits are excluded automatically,
175+
without requiring local cleanup.
176+
- `git log`, `git bisect`, and `git revert` all operate at PR granularity — the
177+
meaningful unit for this project.
178+
- No local history curation required before merging.
179+
180+
**Cons**
181+
182+
- The squashed commit has a new SHA — `git branch -d` does not detect the
183+
original branch as merged.
184+
- Individual commits within the PR cannot be bisected or reverted from main.
185+
- A squash-merged branch should not be reused — start new work from a fresh
186+
branch based on the updated main.
187+
- Detailed development history depends on the GitHub PR record rather than the
188+
cloned repository.
189+
190+
To revert a squash-merged PR, use an ordinary revert:
191+
192+
```
193+
git revert <squash-commit>
194+
```
195+
196+
##### Rebase and merge
197+
198+
GitHub replays each PR commit one by one onto the tip of main, creating new
199+
commits with the same content but new SHAs and updated committer information. No
200+
merge commit is added. The result is a linear history with no structural marker
201+
for the PR boundary.
202+
203+
:::{mermaid}
204+
%%{init: {'themeVariables': {'fontSize': '11px'}}}%%
205+
gitGraph
206+
commit id: "prev"
207+
branch feature
208+
checkout feature
209+
commit id: "implement"
210+
checkout main
211+
commit id: "other PR"
212+
checkout feature
213+
merge main id: "sync main"
214+
commit id: "fix review"
215+
commit id: "fix review 2"
216+
commit id: "nit"
217+
commit id: "address comment"
218+
checkout main
219+
commit id: "implement '"
220+
commit id: "fix review '"
221+
commit id: "fix review 2 '"
222+
commit id: "nit '"
223+
commit id: "address comment '"
224+
:::
225+
226+
After merging, main contains each PR commit recreated as a new commit (new SHA,
227+
same content). History is linear but there is no marker for the PR boundary.
228+
229+
**Pros**
230+
231+
- Linear history, with each individual commit landing on main separately.
232+
- Useful when a PR contains genuinely distinct, independently meaningful
233+
changes.
234+
235+
**Cons**
236+
237+
- All intermediate commits — work-in-progress and "fix findings" — land on main,
238+
requiring the same local curation discipline as create a merge commit.
239+
- `git bisect` may land on intermediate commits that do not build or pass tests.
240+
- Replayed commits get new SHAs — `git branch -d` does not detect the branch as
241+
merged, and original commit signatures are not preserved as verified.
242+
- No structural marker for the PR boundary.
243+
244+
#### Why S-CORE uses squash and merge
245+
246+
The typical S-CORE PR has one substantive commit and several "fix findings"
247+
follow-ups. Those follow-up commits are review artifacts — they exist so
248+
reviewers can see what changed in response to feedback. They carry no
249+
independent value in the permanent history of main.
250+
251+
"Create a merge commit" and "rebase and merge" both land those intermediate
252+
commits on main. Avoiding this would require every contributor to manually
253+
curate their branch before merging. Which contradicts reviewability. Squash and
254+
merge produces the desired result consistently:
255+
256+
- one reviewed change
257+
- one commit on main
258+
- one commit to bisect
259+
- one commit to revert
260+
- one PR containing the detailed review and development history
261+
262+
This separates two concerns: the pull request records how a change was developed
263+
and reviewed; the squash commit records the accepted result in the permanent
264+
project history.
265+
266+
#### Commit message
267+
268+
Because the PR title becomes the squash commit title, it should clearly describe
269+
the resulting change... as a PR title should anyway. The PR reference (e.g.
270+
`(#42)`) as added by GitHub should be retained in the final commit title for
271+
easier tracking.
272+
273+
e.g. *Add Windows platform configuration (#42)*
274+
275+
#### Commit history during review
276+
277+
Contributors are not required to produce a polished commit series inside a PR.
278+
Adding separate commits to address review comments is encouraged — reviewers can
279+
then see exactly what changed since the last review round without re-reading the
280+
full diff.
281+
282+
Before review begins, contributors may freely reorganize their local history.
283+
After review begins, force-pushed history rewrites should be avoided. No local
284+
squashing is required - or desired - before merging. GitHub performs the final
285+
squash after approval.
286+
287+
#### Known limitations
288+
289+
##### Multiple authors
290+
291+
A squash merge replaces the individual PR commits with one new commit on main.
292+
This does not itself change copyright ownership or licensing. However, if
293+
attribution is not preserved correctly, it reduces the authorship and
294+
contribution provenance directly visible in the Git history. This may conflict
295+
with S-CORE's licensing and attribution requirements, including the information
296+
maintained in NOTICE files.
297+
298+
Recommendation: In practice, most PRs have one primary author. When multiple
299+
people contribute to the same PR, preserve their attribution using Git's
300+
Co-authored-by trailers. GitHub normally adds these automatically when squashing
301+
commits from multiple authors, but the final commit message should still be
302+
verified before merging.
303+
304+
Working on the same feature does not automatically mean that copyright is
305+
shared. Minor, purely mechanical changes, such as correcting a typo, will often
306+
not constitute a separately copyright-protected contribution.
307+
308+
Existing copyright, DCO, CLA, licensing, and internal IP requirements still
309+
apply. If attribution cannot be represented adequately in one squash commit,
310+
resolve this before merging or split the work into separate PRs.
311+
312+
*Note: We have discussed this with lawyers, but we are not lawyers, and this
313+
documentation is not legal advice. If you have questions about licensing,
314+
copyright, or other legal matters, consult a qualified legal professional.*
315+
316+
##### Distinct topics
317+
318+
A PR should represent one coherent change.
319+
When changes need to remain independently identifiable, revertible, or
320+
releasable, submit them as separate PRs instead.
321+
58322
---
59323

60324
## 1.4 Repository Standards 🟠

0 commit comments

Comments
 (0)