Skip to content

feat: auto-tolerate version-only package.json edits in checkchange#7536

Closed
janechu wants to merge 1 commit into
mainfrom
users/janechu/prebump-fast-changes
Closed

feat: auto-tolerate version-only package.json edits in checkchange#7536
janechu wants to merge 1 commit into
mainfrom
users/janechu/prebump-fast-changes

Conversation

@janechu
Copy link
Copy Markdown
Collaborator

@janechu janechu commented May 28, 2026

Pull Request

📖 Description

Add build/scripts/checkchange.mjs, a thin wrapper around beachball check that auto-tolerates version-only hand edits to packages/*/package.json.

Today npm run checkchange fails for any package.json edit in a publishable workspace — including a single-line version bump — because beachball requires a corresponding change file. FAST does enough hand-bumping (hotfix overrides, paired Rust/npm sync recovery, automation-driven version pins) that requiring a no-op change file by hand each time is friction, and instructing maintainers to never edit package.json versions by hand (the previous CONTRIBUTING.md guidance) is brittle.

The wrapper inspects the branch + staged diff and, for each packages/<pkg>/package.json whose only branch diff is a single-line "version" field bump (no other files in the package directory, no other fields in package.json, no existing change file for the package), adds --scope "!packages/<pkg>" to the beachball check invocation. Beachball then treats that package as out of scope and does not demand a change file. Any other edit in the package directory re-enables the standard requirement.

Documentation in CONTRIBUTING.md and .github/skills/shipping/SKILL.md is updated to describe the carve-out, including the reminder that hand edits to a paired Rust crate's Cargo.toml/Cargo.lock still have to be done manually (the postbump hook only fires during npm run bump).

👩‍💻 Reviewer Notes

  • Why scope exclusion, not auto-generated change files? Beachball's "package already has a change file" lookup is a git diff --diff-filter=A against the target branch, so a generated change/*.json only counts if it's committed. A check wrapper mutating git state on every invocation would be unacceptable. Scope exclusion is purely in-process, leaves no artifacts, and is auditable through the verbose log the script prints before exec'ing beachball.
  • Auto-exclusion conditions (all must hold, see the script header for the full rationale):
    1. The package directory has exactly one changed file: package.json.
    2. The package.json diff is exactly one removed line and one added line, both matching the "version": "..." field, old != new.
    3. The package is not "private": true.
    4. No change file for the package already exists in change/.
  • Bump-PR behaviour is intentionally unchanged. Bump PRs touch CHANGELOG.md and frequently dependency versions in package.json, so they fail conditions 1 and 2 and are not auto-excluded by this wrapper. That keeps this PR's scope narrow.
  • The wrapper honours the existing --scope "!sites/*" and --changehint exactly as the old script did, and forwards extra args via process.argv.slice(2).

📑 Test Plan

Verified locally against a fresh npm ci:

  • npm run checkchange on a clean tree → no-op, exits 0.
  • Hand-edit packages/fast-element/package.json version only (staged) → wrapper logs [checkchange] Auto-excluding @microsoft/fast-element …, beachball check exits 0.
  • Hand-edit version + a source file in the same package (packages/fast-element/src/index.ts) → wrapper does NOT exclude, beachball check fails with Change files are needed! and the original hint.
  • Hand-edit a non-version field in package.json (with or without a version bump in the same diff) → wrapper does NOT exclude, beachball check fails as before.
  • Staged change file already present for the bumped package → wrapper does NOT exclude (lets beachball evaluate the change file normally).
  • npx biome check build/scripts/checkchange.mjs beachball.config.js → no issues.
  • npm run format:check → passes.

✅ Checklist

General

  • I have included a change request file using $ npm run change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Note on the change-file box: this PR's changes live entirely in .github/ (skill), root package.json (script wiring), beachball.config.js, build/ (script), and CONTRIBUTING.md. None of those paths are publishable workspaces, so beachball does not require a change file for this PR (and the wrapper introduced here is itself the mechanism that confirms that).

Note on the tests box: the changes are exercised end-to-end through npm run checkchange against synthetic manual-bump diffs (see Test Plan). The wrapper has no separately published surface, so dedicated unit tests are not introduced.

⏭ Next Steps

A natural follow-up is to extend the wrapper (or write a sibling helper) to handle the bump-PR scenario itself — today the bump PR fails npm run checkchange because git diff --diff-filter=A does not surface the deleted, just-consumed change files (despite the current note in CONTRIBUTING.md step 5 claiming the bump PR passes). That fix is out of scope for this PR but should be tracked separately.

Add build/scripts/checkchange.mjs, a thin wrapper around 'beachball check'
that scans the branch + staged diff for packages/*/package.json files
whose only change is a single-line 'version' field bump (no other files
in the package directory, no other package.json fields touched, no
existing change file). For each such package the wrapper adds
'--scope "!packages/<pkg>"' to the beachball invocation, so beachball
treats the package as out of scope and does not demand a change file.

Wire the wrapper into the existing 'checkchange' npm script and update
CONTRIBUTING.md plus the shipping skill to document the carve-out for
hotfix overrides, paired Rust/npm sync recovery, and automation-driven
version pins.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@janechu
Copy link
Copy Markdown
Collaborator Author

janechu commented May 28, 2026

Superseded by a follow-up that uses a branch-name + author allowlist instead of diff-content inspection. See discussion for context — closing in favor of the new, narrower PR.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant