diff --git a/skills/git-workflow/references/advanced-git.md b/skills/git-workflow/references/advanced-git.md index e334fdf..ff4794e 100644 --- a/skills/git-workflow/references/advanced-git.md +++ b/skills/git-workflow/references/advanced-git.md @@ -909,6 +909,68 @@ To compare a file across branches without checkout, use `git diff -- ` (or `git diff ... -- ` for the merge-base–relative diff). +### Never `git checkout -- ` while the change is uncommitted + +`git checkout -- ` overwrites the working-tree file **without +warning and without a reflog entry**. Uncommitted content is never written to +the object store, so there is usually nothing in Git to recover from — only an +editor's local history or a filesystem snapshot can still hold it. The pattern +that bites is comparing current behaviour against a baseline: + +```bash +git checkout -- src/Service.php # measure the old behaviour +# … run the probe … +git checkout HEAD -- src/Service.php # "restore" +``` + +That last line restores the file as it is **committed**. If the fix you were +measuring was still only in the working tree, it is now gone, silently, and the +next `git add` commits the file without it. Observed cost: a commit pushed +without the fix it was created for, discovered only because the test output was +read line by line afterwards. + +Use `git show :` (above) to read the baseline instead — it never +touches the working tree. Where a baseline must genuinely be *on disk* (an +autoloader, a bundler), commit or stash first, or check the baseline out into a +throwaway worktree: + +```bash +git worktree add /tmp/baseline +``` + +### Never chain `git push` behind a test run with `&&` + +```bash +composer test | grep -E 'OK|FAILURES' && git add -A && git commit -s -m … && git push +``` + +This pushes on a failing suite. `&&` propagates the exit status of the **last** +command in the pipeline, and `grep` exits 0 as soon as it matches *anything* — +including the word `FAILURES`. Filtering test output for readability discards +the very status the chain depends on. + +Run the suite as its own command, read the result, and only then commit and +push. If it must be one invocation, gate on the runner rather than the filter — +capture the runner's status explicitly, which works in any POSIX shell: + +```bash +composer test > out.txt; rc=$? +grep -E 'OK|FAILURES' out.txt # read it, but don't gate on it +[ "$rc" -eq 0 ] || exit 1 +``` + +(`set -o pipefail` does the same in bash, zsh and ksh, but it is not in POSIX +`sh` — a script with `#!/bin/sh` under dash will fail on it.) + +Afterwards verify what actually landed, on the remote rather than locally. +Match one unique line from the change — `grep -c` counts matching *lines*, so a +multi-line pattern will not match at all; `-F` avoids regex surprises in code: + +```bash +git fetch +git show FETCH_HEAD: | grep -cF '' +``` + ## Troubleshooting ### Common Issues @@ -975,4 +1037,3 @@ gh api repos/OWNER/REPO/compare/... \ Verify against `origin` (or the compare API) before deleting a tag, choosing a release version, or concluding two lines forked — not against local refs. - diff --git a/skills/git-workflow/references/pull-request-workflow.md b/skills/git-workflow/references/pull-request-workflow.md index f349892..11cca7d 100644 --- a/skills/git-workflow/references/pull-request-workflow.md +++ b/skills/git-workflow/references/pull-request-workflow.md @@ -828,6 +828,54 @@ jobs: ## Conflict Resolution +### Two independent PRs needing the same hunk — make it byte-identical + +When two PRs split out of one piece of work both need the same addition (a +`catch` block both paths now reach, an enum case both use, the same import), +there are three options and only one of them is good: + +| Approach | Cost | +|---|---| +| Stack the second PR on the first | Second PR can no longer merge on its own | +| Put the hunk in one PR only | The other PR is broken until that one lands — a merge-order trap | +| **Put the identical hunk in both** | **Git merges it once, in any order** | + +Git treats the same change at the same place as *one* change, so both PRs merge +cleanly and the hunk appears exactly once in the result. That removes the +ordering constraint entirely, which matters because maintainers merge in +whatever order they review. + +It only works if the text matches **byte for byte** — same wording, same +indentation, same position, and the same import placement (run the project's +formatter in both branches, or the import sorter will reorder one of them). Copy +it mechanically rather than retyping, and compare the two extracts. `` +and `` are the first and last lines of the hunk, used as `sed` +address patterns: + +```bash +git show : | sed -n '//,//p' > /tmp/hunk +diff <(sed -n '//,//p' ) /tmp/hunk && echo identical +``` + +Then prove the property instead of assuming it — merge every order in a +throwaway clone and check the result. Abort the run on the first conflict: +continuing would leave the repo mid-merge and every later result meaningless. + +```bash +git clone -q --shared /tmp/mergetest && cd /tmp/mergetest +for order in " " " "; do + git reset -q --hard + for b in $order; do + git merge -q --no-edit "$b" || { echo "CONFLICT: $b in order [$order]"; git merge --abort; exit 1; } + done +done +# counts matching LINES, so use one distinctive line of the hunk, not the hunk +grep -cF '' # must be 1, not 2 +``` + +State in both PR bodies that the hunk is identical and why, so a reviewer does +not "clean up" the duplicate in one of them. + ### Before Merging ```bash