chore: use the standard fork remote layout - #55
Merged
Merged
Conversation
Everything in this file that warned about remote names, spelled commands with full URLs, or repeated that `FETCH_HEAD` holds only the last fetch existed because this clone had `origin` pointing at upstream and the fork on a second remote named `fork` — the reverse of the convention. That is a solved problem, and I wrote a workaround for it instead of fixing it. The workaround then produced its own bugs: three wrong instructions from `FETCH_HEAD` being last-fetch-wins, and a verify step that compared our main against our own fork and so could not fail. The file now states the standard layout up front -- `origin` ours, `upstream` theirs -- with the rename commands to get there, and uses `origin/main` and `upstream/main` throughout. Those are stable refs, so the ordering hazard disappears rather than needing a warning at each site. 153 lines to 128, and `FETCH_HEAD` from eleven mentions to one. Every command in the file was executed against this repo. That found `git fetch origin upstream`, which does not fetch two remotes -- git reads `upstream` as a refspec on `origin` and fails with "couldn't find remote ref upstream". It is `git fetch --multiple origin upstream`.
anoop-narang
requested review from
shefeek-jinnah
and removed request for
a team
September 28, 2026 05:34
The setup block assumed a clone made from upstream and opened with `git remote rename origin upstream`. Run against a clone of this fork, whose `origin` is already correct, that renames the right remote away. Worse, it fails silently. Running part of the block on a fork clone — the rename and the add, without the `set-url` that used to follow — leaves both remotes pointing at this fork. `upstream/main` then means our own `main`, so the sync merges nothing, the post-sync check passes, and the branch guard sees no fork commits. Every recipe here reports success while comparing us against ourselves. Now split by what the clone actually is, with `git remote -v` first: a clone of the fork adds `upstream`, a clone of upstream renames and adds `origin`. Both paths run from scratch in a throwaway repo and end correct; the previous block left case one wrong. Adds the check that matters, since the failure has no symptom: the two remotes must name different repositories.
`git remote rename` rewrites the tracking config of every branch that followed the renamed remote. The documented rename therefore leaves local `main` following `upstream/main`: a `git pull` on `main` merges upstream straight into it, and nothing says so. Verified in a throwaway repo — before the rename `main` tracks `origin`, after it tracks `upstream`. The recipe now fetches and re-points `main` at `origin/main` afterwards. The opening line had the same weakness from the other direction. It claimed `git merge-base main upstream/main` resolves, which rests on local `main` being both correctly tracked and current — and with the tracking bug above it would pass whatever the fork's state was. It now names `origin/main`, and says why: a local branch can track the wrong remote or be stale, and neither condition announces itself. This repo escaped the tracking bug by ordering luck. I re-pointed `main` at the fork before renaming the remotes, so the rename rewrote it to `origin` rather than away from it.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📊 Benchmark ComparisonCurrent:
Compared Liquid vs DataFusionDefault on the same runner |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Everything in
CLAUDE.mdthat warned about remote names, spelled commands with full URLs, or repeated thatFETCH_HEADholds only the last fetch existed for one reason: this clone hadoriginpointing at upstream, with our fork on a second remote namedfork— the reverse of the convention.That is a solved problem. I wrote a workaround instead of fixing it, and the workaround produced its own bugs — three wrong instructions from
FETCH_HEADbeing last-fetch-wins, and a verify step that compared our main against our own fork and so could not fail whatever the sync state.The fix
The layout every tutorial and every agent already expects.
CLAUDE.mdnow states it up front with the rename commands, and usesorigin/mainandupstream/mainthroughout. Those are stable refs, so the ordering hazard disappears instead of needing a warning at each site.The deleted material is not lost guidance — it was scaffolding holding up a broken foundation.
Every command was executed
Which is how I found one that never worked:
git fetch origin upstreamdoes not fetch two remotes — git readsupstreamas a refspec onorigin. It isgit fetch --multiple origin upstream. That line was in the sync recipe and the verify step.Full run against this repo:
The guard workflow needs no change: in Actions,
actions/checkoutsetsoriginto the repo being built, which is this fork either way.