close
Skip to content

Ban the word "legacy" in code comments and commit messages - #1134

Open
mark-sil wants to merge 1 commit into
mainfrom
legacy-term-rule
Open

Ban the word "legacy" in code comments and commit messages#1134
mark-sil wants to merge 1 commit into
mainfrom
legacy-term-rule

Conversation

@mark-sil

@mark-sil mark-sil commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

The code-comment standard allowed "legacy X" as a behavioral-parity WHY. Once the code it refers to is deleted, the word names nothing a reader can look up, and they cannot tell whether the behavior is still required.

Ban the term in comments and in commit messages, which are both read long after the referent is gone. PR descriptions and review discussion stay exempt: they are read alongside the diff while the context is live, so the team's own word is the clearest one there.

State the scope in AGENTS.md, where it is in context regardless of which skill fires, then repeat the relevant half where each artifact is authored -- the two skills for the ban, pr-pitch for the exemption, so a PR body is not written under a rule that does not apply to it.


This change is Reviewable

The code-comment standard allowed "legacy X" as a behavioral-parity
WHY. Once the code it refers to is deleted, the word names nothing a
reader can look up, and they cannot tell whether the behavior is still
required.

Ban the term in comments and in commit messages, which are both read
long after the referent is gone. PR descriptions and review discussion
stay exempt: they are read alongside the diff while the context is
live, so the team's own word is the clearest one there.

State the scope in AGENTS.md, where it is in context regardless of
which skill fires, then repeat the relevant half where each artifact is
authored -- the two skills for the ban, pr-pitch for the exemption, so
a PR body is not written under a rule that does not apply to it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

NUnit Tests

    1 files  ±0      1 suites  ±0   12m 40s ⏱️ -9s
6 084 tests ±0  5 999 ✅ ±0  85 💤 ±0  0 ❌ ±0 
6 093 runs  ±0  6 008 ✅ ±0  85 💤 ±0  0 ❌ ±0 

Results for commit b13280f. ± Comparison against base commit cbb5dc2.

@papeh papeh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG overall. One change request.

This covers the assertion messages of new tests too. Commit messages
follow the same rule (`.claude/skills/commit-messages/SKILL.md`). PR
descriptions and review discussion are exempt, so do not coin a
euphemism to avoid the word there.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to coin a euphemism anywhere. "PR Descriptions and review discussions are exempt from this rule" should suffice.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can remove it, but my preference would be to leave it. Because of the ban in other places, Claude thought it would be best to use the word "classic' instead of "legacy" in a PR I was working on. Telling Claude not to "coin a
euphemism" was Claude's suggestion as a way to prevent it from doing that.

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.

2 participants