lint: warn on long comment blocks; trim three that landed this week - #494
Open
pcarleton wants to merge 4 commits into
Open
lint: warn on long comment blocks; trim three that landed this week#494pcarleton wants to merge 4 commits into
pcarleton wants to merge 4 commits into
Conversation
…e norm in AGENTS.md Comment blocks over 15 lines (25 for a file header) get a warning whose text tells the author — human or agent — to keep the rule and the non-obvious why in the comment and move history/process notes to the PR description. Implemented as an inline local rule in eslint.config.mjs (no new dependency); vendored src/spec-types are exempt; the ten pre-existing long comments that are genuine API/scenario contracts carry a targeted disable with a reason. Warnings do not fail npm run lint / CI; setup-node's eslint-stylish problem matcher turns them into PR annotations. Co-Authored-By: Claude <noreply@anthropic.com>
… why Comment-only change. Drops review-time narration (which sites had drifted, what the pre-fix behaviour was) and keeps the rule plus the non-obvious reason, per AGENTS.md. Co-Authored-By: Claude <noreply@anthropic.com>
commit: |
pcarleton
marked this pull request as ready for review
September 6, 2026 20:16
pcarleton
enabled auto-merge (squash)
September 6, 2026 20:17
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.
Motivation and Context
Review bandwidth. A growing share of PRs here are AI-assisted, and they tend to arrive with 20 to 30 line comment blocks that narrate spec history, process and review context: material that belongs in the PR description, not in source we maintain forever. Recent examples: the fixture header and
tools.tsblock in #380 (trimmed before merge) and three blocks that landed with #476 and #483 (trimmed here). A lint warning with an actionable message lets the author's tooling fix this before a maintainer has to ask, and stating the norm in AGENTS.md reaches agents before they write the code.What
local/comment-length, an inline rule ineslint.config.mjs(no new dependency): warns when a block comment or a run of//lines exceeds 15 lines (25 for a file's header comment). The message says what to do: keep the rule and the non-obvious "why", move history and process notes to the PR description. Vendoredsrc/spec-types/**is exempt; nine pre-existing long comments that are genuine API or scenario contracts carry a targeted disable with a reason.local/comment-lengthwarning means "move this to the PR description", not "add a disable".http-base.ts,issuer-parameter.tsandresource-mismatch.ts(from fix(http-base): return valid empty results for unhandled list methods #476 and fix(client/auth): bind negative-check verdicts to an observed prerequisite #483) down to the standalone why. Comment-only, no behaviour change.warn:npm run lintand CI stay green (exit 0), andactions/setup-node's ESLint problem matcher shows the warnings as inline annotations on the PR diff.Not included: an optional follow-up that posts one sticky PR comment listing the warnings (two-workflow
workflow_runpattern, since fork PR tokens are read-only). Happy to add it if we want a commenting bot; the annotation path may be enough.How Has This Been Tested?
npm run checkclean with zero warnings on the branch; client auth and http-base tests pass (85). Onmainthe rule finds 18 body / 8 header sites at these thresholds (all handled here); run against #380's original head it flags exactly the two blocks that were trimmed by hand.Breaking Changes
None. Lint warnings only.
Types of changes
Checklist