Skip to content

chore(knip): separate obsolete exclusions and export-tag cleanup - #1870

Open
DenizOkcu wants to merge 1 commit into
Nano-Collective:mainfrom
DenizOkcu:chore/knip-cleanup
Open

DenizOkcu wants to merge 1 commit into
Nano-Collective:mainfrom
DenizOkcu:chore/knip-cleanup

Conversation

@DenizOkcu

Copy link
Copy Markdown
Contributor

Why

Split from #1570 following @addyCooks’ request for smaller PRs. This isolates unrelated scope (d) and is independent of the Ink upgrade.

Changes

  • Remove obsolete ignored binaries and the broad .github/** exclusion.
  • Remove stale @internal annotations from exports used by configuration or tests.

Validation

Whitespace checks pass. No runtime behavior changes intended; branch CI should run knip and typechecking.

@github-actions

Copy link
Copy Markdown
Contributor

No changeset found

This PR does not add a changeset, so it will not appear in the changelog or trigger a release.

If the change is user-facing, add one:

pnpm changeset

Pick a bump (patch / minor / major) and write the changelog entry in our usual voice ("Added X... Thanks to @you. Closes #123."), then commit the generated .changeset/*.md file.

If this PR is docs-only or a chore that needs no release note, you can ignore this — or run pnpm changeset --empty to record that intentionally.

@github-actions

Copy link
Copy Markdown
Contributor

nc-review: comments — 1 nit

@DenizOkcu — a few things worth a look, none blocking.

This PR tightens the knip configuration by dropping four ignoreBinaries entries that were dead weight and removing the blanket .github/** exclusion, then strips now-inaccurate @internal tags from four exports. I verified each half against the base tree: the removed binaries never appear in package.json scripts (knip's only source for unused-binary detection), .github contains no JS/TS for knip to newly analyse, and all four de-tagged exports have real consumers (three spec files, plus DiskModeProviders referenced in-file under ignoreExportsUsedInFile.type). The change is correct and low-risk; the only thing I would ask for is that the surviving @internal tags stay accurate, since two of the four removals leave the section comment and the sibling tags making a claim this PR partly invalidates.

⚪ nit · design · source/types/config.ts:333

After this PR, DiskModeProviders is the only type in the Disk* block with its @internal tag removed, while DiskNanocoderConfig and DiskConfig keep theirs — yet the block header at line 333 still states:

// These types are consumed exclusively by that tooling and are not part of
// the runtime surface, hence @internal.

DiskModeProviders is still consumed exclusively by the schema tooling, so the tag was arguably correct in the first place and the removal was driven by knip rather than by the type's actual role. That is fine — ignoreExportsUsedInFile.type is what actually covers it, so knip stays green either way — but the header now describes a block that is only partly tagged, and the two retained @internal tags on DiskNanocoderConfig/DiskConfig are what a future reader will trust. Consider either leaving DiskModeProviders tagged and adding DiskModeProviders to an explicit ignore, or dropping the remaining two tags and rewording the header so the block tells one consistent story. Purely cosmetic; not worth blocking on.


🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional

Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.

@github-actions github-actions Bot added the agent:comments nc-review left non-blocking findings label Oct 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Hi @DenizOkcu, thanks for this PR! It looks like a maintainer has left feedback
or review activity and there are still some outstanding items to wrap up.

Whenever you get a chance, could you take a look at the open comments?
If anything is unclear or you'd like a hand, just reply here and we'll help you get it across the line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:comments nc-review left non-blocking findings stale:nudged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant