Fix stale blame-ignore hash and add-a-package lint step - #806
Merged
Conversation
…on main The recorded hash named the commit as it existed on its own branch. Rebase-merging that branch onto main rewrote it, so blame resolved nothing and the whole-tree reformat was blameable again with no signal that it was. The note above the entry blamed local rebases alone and suggested matching by subject line, which is what let this recur: a stale hash still names a real commit in the reflog, so grepping the log finds a plausible answer while blame silently ignores nothing. It now names the merge as a rewrite too, and gives an ancestry check that actually fails when the entry is wrong.
The Worker-isomorphism step still told a new package to copy a sibling's eslint.config.ts and hand-roll the no-restricted-imports/no-restricted-globals Node ban -- the duplication packageLintConfig() exists to remove. Every package already calls the helper, so this was documentation describing a pattern the code had left behind. The step now names the helper and the isomorphic flag, and says where package-specific rules belong: the helper's own options rather than a second config block, because flat config replaces a same-key rule instead of merging it, so a hand-written no-restricted-imports switches the isomorphism ban back off while still reporting itself as working.
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
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.
Two independent gaps found on main at
8a70023f, one commit each..git-blame-ignore-revsnamed a rewritten commitThe recorded hash was the reformat commit as it existed on its own branch. Rebase-merging that branch onto main rewrote it, so
git merge-base --is-ancestor <sha> HEADfails and blame resolves nothing — the 1191-file whole-tree reformat was blameable again with nothing to signal it.Now points at
c12e7402, verified an ancestor of HEAD. I checked it's the onlystyle: format the workspacecommit in current history, so there's no ambiguity about which one it should be.The note above the entry is also corrected, because it's what let this recur: it blamed local rebases alone, and suggested matching by subject line. A stale hash still names a real commit in the reflog, so grepping the log finds a plausible-looking answer while blame silently ignores nothing. The note now calls out the merge as a rewrite too and gives the ancestry check, which actually fails when the entry is wrong.
The add-a-package lint step described the pre-refactor pattern
Step 4 told a new package to give itself an
eslint.config.tscarrying theno-restricted-imports/no-restricted-globalsNode ban, "using a sibling's own config as a template" — the exact hand-rolled duplicationpackageLintConfig()was built to remove. Pure doc drift: every package already calls the helper.Rewritten to name the helper and the
isomorphicflag, and to say where package-specific rules belong — the helper's own options, not a second config block. That last part is the non-obvious bit worth writing down: flat config replaces a same-key rule rather than merging it, so a hand-writtenno-restricted-importssilently switches the isomorphism ban back off while still reporting itself as working.markdown-codec's config already carries a comment saying exactly this, learned the hard way.I verified every option name cited (
additionalRestrictedImportPatterns,additionalIgnores,barrelPolicy,nonNullAssertion) exists onPackageLintOptions, so the fix doesn't introduce fresh drift.Also checked, not changed
Step 5 (
_typecheck:attw) — accurate: twelve packages define the script and each one's owntypecheckruns it through turbo. Steps 1-3, 6, 7 unchanged.The two packages that still mention
no-restricted-imports/no-restricted-globals(markdown-codec,documents) are legitimate: both callpackageLintConfig, and their extra rules are a markdown-library ban routed through the helper's options and the web UI's RPC import boundary respectively.