Make the comment rule's test explicit: delete, don't shorten - #358
Merged
Conversation
Three passes over one PR shortened comments that should not have existed. The rule already said what earns a comment; what it did not say is how to check, and 'can I justify this?' keeps essays that 'what is lost without it?' removes.
sibson
added a commit
that referenced
this pull request
Aug 17, 2026
The comments rule has now been amended four times in four days -- ba31647 added it, #365 banned task references, #358 made the delete test explicit, and the last commit here swapped in a silent-failure test. Each amendment blacklisted whatever the previous violation looked like, so each one was escapable by writing a differently-shaped bad comment, which is what kept happening. Every PR in this stretch has needed a trim pass. The silent-failure test was also wrong on its own terms. Two of the three comments that have survived untouched in loggingproxy.py describe loud failures -- a desync and a truncated capture -- and they are the best comments in the repo. What separates them from the trimmed ones is not the failure mode but the information: they say something an intelligent reader could not have known and cannot look up. That is the admission test now, and the prohibitions collapse into a clause, since restating an option's name and re-telling the branch's history both fail it for the same reason. Shorter than what it replaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Your rewrite of the comment rule already covers what I got wrong — docstrings named, one or two lines as the norm, no narrating what a change replaced, rationale to
docs/. I still needed three passes over one PR, so this adds the one thing missing: the check, rather than the standard.I was asking can I justify this comment? and keeping anything that passed. The question that actually removes them is what does the reader lose without it? — which is why pass two kept five-line explanations and pass three shortened them instead of deleting them.
Four lines, no restructuring.
🤖 Generated with Claude Code