Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions .claude/commands/commit.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,36 @@ type(scope): imperative subject

BREAKING CHANGE: <only if applicable>

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
```

- **type**: one of `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`, `revert`. `commitlint.config.js` extends [`@commitlint/config-conventional`](https://www.npmjs.com/package/@commitlint/config-conventional), which defines the allowed set — pick the type that genuinely matches the change (`feat`/`fix` only for actual features/bug fixes).
- **scope**: full package name (`ui-button`, `ui-select`). Comma-separate for a few, use `many` for several, omit for repo-wide.
- **subject**: imperative ("add loading state", not "added"). Must start with a lowercase letter (commitlint's `subject-case` rejects sentence/Start/PascalCase). No trailing period.
- **Body lines: hard-wrap at 100 characters.** Commitlint (`body-max-line-length: 100`) runs in CI and will reject longer lines. The footer lines (Claude Code attribution, Co-Authored-By) are exempt.
- **subject**: imperative ("add loading state", not "added"). Must start with a lowercase letter (commitlint's `subject-case` rejects sentence/Start/PascalCase). No trailing period. **Max 72 characters** (`subject-max-length`) — if it doesn't fit, you're listing everything the change touches instead of naming the change.
- **Breaking changes**: add a `BREAKING CHANGE:` line in the body describing what breaks. See CLAUDE.md for what counts as breaking.
- **Attribution**: end with `Co-Authored-By: Claude <noreply@anthropic.com>` — that exact form, not a model-specific one, so history stays consistent. **No `🤖 Generated with` line in commit messages**; that belongs in PR bodies (`/pr` handles it).

### Body

**Omit the body when the subject says it all.** When you do write one, it explains **why** — the constraint, the cause, the thing the diff cannot show. Never restate what changed.

- **Hard-wrap at 100 characters** (`body-max-line-length`). Trailers are exempt.
- ❌ **Never turn the body into a changelog.** No grouping headings (`Configuration:`, `Build Tooling:`), no numbered sections, no long bullet list of the files you touched. Commitlint's `body-no-changelog` rejects 2+ headings, more than 12 bullets, or more than 6 bullets naming files.
- ✅ A single lead-in like `The fixes:` followed by a few bullets is fine, and naming a specific file is fine when the file _is_ the point.
- Hard ceiling of 28 body lines (`body-max-lines`) — a backstop for runaway bodies, not a target.

```
❌ feat(many): migrate from npm to pnpm ✅ feat(many): migrate from npm to pnpm

Configuration: regression-test stays on npm so it keeps
- Add pnpm-workspace.yaml installing @instructure/ui the way an
- Add .npmrc with hoisted node linker external consumer would.
Build Tooling:
- Update scripts/bootstrap.js
...25 more lines...
```

Writing about _before_ and _after_ is encouraged — "Previously the placeholder only showed on hover" is exactly right in a commit message, which is permanently anchored to its own diff. (Code comments are different: see CLAUDE.md.)

## Steps

Expand Down
68 changes: 68 additions & 0 deletions .claude/commands/tidy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
---
description: Tighten comment and commit message wording in the working diff
---

Review the wording of comments and commit messages on this branch and tighten anything that
breaks the rules. This is a **wording-only** pass - do not change behaviour, rename anything,
or restructure code. For code quality use `/simplify`, for bugs use `/code-review`.

## The rules being checked

Comment rules: the "Code Comments" section of `CLAUDE.md`.
Commit message rules: `.claude/commands/commit.md`.

The single test both share: **would this still make sense to someone reading it in a year, who
never saw this branch, the PR, the ticket, or the conversation that produced it?**

## Process

1. **Collect the changes**

```
git diff
git diff --cached
git log master..HEAD --format='%H%n%s%n%n%b%n---'
```

If the branch has no commits and no working changes, say so and stop.

2. **Check added and modified comments.** Only lines this branch touched - read the diff, not
whole files. Flag:

- comments that restate what the code does
- comments longer than they need to be, or split over several lines for no reason
- references to the change itself: `now`, `new`, `previously`, `used to`, `this change`,
`the fix`, `as discussed`, `per review`, `we decided`, `recently`
- narration of the diff: `// added onKeyDown handler`, `// updated to support X`
- trailing comments on the same line as code, and comments below what they describe
- commented-out code, decorative separators, banner comments
- prop JSDoc carrying `@param`/`@type` tags, or running longer than a sentence
- **comments added to code the branch did not otherwise change**

3. **Check commit messages** against the `/commit` rules: subject over 72 characters, a body
that restates the diff, a changelog shape (several grouping headings, a long bullet list,
bullets enumerating changed files), or a `🤖 Generated with` line.

Do **not** flag before/after wording here. "Previously the placeholder only showed on
hover" is correct in a commit message — the message is permanently attached to its own
diff. That rule exists for comments, which persist with no such anchor.

4. **Report before changing anything.** One table or list, grouped into comments and commit
messages, each entry `file:line` (or the commit's short sha) with the current text and the
proposed replacement. Say plainly if there is nothing to fix.

5. **Apply on confirmation.**

- Comment rewrites: edit the files directly. Leave the working tree staged as you found it.
- Commit message rewrites: these rewrite history, so **always confirm separately** and tell
the user it will change the shas. Amend with `git commit --amend` for `HEAD` only; for
older commits, explain that a rebase is needed and let the user decide whether it is
worth it. **Never rewrite history without an explicit go-ahead**, and never on a branch
that has been pushed and reviewed unless the user says so.

## Important

- Deleting a comment is usually the right fix. Do not rewrite a comment that should not exist.
- Do not touch the MIT license header block at the top of every file.
- Do not add new comments. This pass only shortens and removes.
- Do not touch comments outside the diff, however tempting.
13 changes: 13 additions & 0 deletions .claude/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,18 @@
"Bash(gh --help)",
"Bash(gh * --help)"
]
},
"hooks": {
"PreToolUse": [
{
"matcher": "Bash",
"hooks": [
{
"type": "command",
"command": "node \"$CLAUDE_PROJECT_DIR/scripts/claude/check-commit-message.mjs\""
}
]
}
]
}
}
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ CLAUDE.local.md
!.claude/commands/slack-setup.md
!.claude/commands/implement.md
!.claude/commands/ticket.md
!.claude/commands/tidy.md

# Playwright MCP
.playwright-mcp
25 changes: 25 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,31 @@ External docs (preferred over guessing component APIs): https://instructure.desi
- **New components: functional + hooks only.** Class components exist in legacy code — don't extend that pattern.
- Styling is Emotion CSS-in-JS via `theme.ts` files co-located with each component.

## Code comments

**A comment must make sense to someone reading the file in a year who never saw the change that introduced it** — no access to the PR, the ticket, or the conversation. The same standard applies to commit messages; `/commit` has the specifics.

- Explain **why**, never **what**. If the code already says it, delete the comment.
- **One line.** Two or three only when the reason genuinely needs them.
- **Never reference the change itself.** `now`, `new`, `previously`, `used to`, `this change`, `the fix`, `as discussed`, `per review`, `we decided`, `recently` — these only mean something next to the diff. Name the constraint instead.
- **Never narrate the diff** (`// added onKeyDown handler`, `// updated to support X`) — that's what `git log` is for.
- No commented-out code, no banner or separator comments, and **don't add comments to code you didn't change**.
- Lowercase `//` on its own line above what it explains, never trailing. Ticket ids only on a real external blocker: `// TODO INSTUI-1234: <what unblocks it>`.
- Leave the MIT license header alone — `notice/notice` in `eslint.config.mjs` enforces it.

```ts
// ❌ verbose, references the change, restates the code
// We now memoize this because we found a performance issue during testing
// where the component re-rendered too often. Previously computed inline.
const styles = useMemo(...)

// ✅ names the constraint, reads standalone
// getCSSStyleDeclaration costs ~100ms per call
const styles = useMemo(...)
```

Prop docs are a JSDoc block with **one prose sentence** and no `@param`/`@type` — types come from TypeScript and `react-docgen`. See `packages/ui-alerts/src/Alert/props.ts`.

## Component versioning (v1/v2)

Some components ship in two versions during a migration period — a legacy **v1** and a newer **v2** (e.g. `DateInput`). v2 is the preferred implementation for new work; v1 is deprecated and gets removed in a later major release. Don't assume a component has only one version: check its README and the package exports to see which versions exist and which is current before using or changing one.
Expand Down
88 changes: 83 additions & 5 deletions commitlint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,94 @@ function getAllPackages() {
}
}

// Trailers and ticket ids carry no prose, so they don't count towards the body
// limits.
const isTrailer = (line) =>
/^[A-Za-z][A-Za-z-]*:\s/.test(line) ||
/^[A-Z][A-Z0-9]+-\d+$/.test(line) ||
line.startsWith('🤖')

function bodyLines(raw) {
return (raw || '')
.split('\n')
.slice(1)
.map((line) => line.trim())
.filter((line) => line !== '' && !line.startsWith('#') && !isTrailer(line))
}

/**
* Caps body length. Set well above the longest real commit so it only catches a
* body that has turned into a full changelog of the diff.
*/
function bodyMaxLines(parsed, _when, max) {
const count = bodyLines(parsed.raw).length
return [
count <= max,
`body has ${count} lines, the limit is ${max}. Explain why the change was ` +
'made; the diff already covers what changed.'
]
}

/**
* Rejects a body shaped like a changelog: grouped under several headings, or a
* long bullet list naming the files that changed. Thresholds sit above the
* heaviest legitimate usage in this repo - one "The fixes:" style lead-in and a
* handful of bullets are fine.
*/
function bodyNoChangelog(parsed) {
const lines = bodyLines(parsed.raw)
const headings = lines.filter((line) =>
/^[A-Z][A-Za-z /()]{2,40}:$/.test(line)
)
const bullets = lines.filter((line) => /^[-*] /.test(line))
const pathBullets = bullets.filter((line) =>
/(packages\/|scripts\/|\.(ts|tsx|js|jsx|mjs|cjs|json|ya?ml|md)\b)/.test(
line
)
)

if (headings.length > 1) {
return [
false,
`body groups changes under ${headings.length} headings (${headings.join(
' '
)}). ` + 'Write prose explaining why, not a grouped changelog.'
]
}
if (bullets.length > 12) {
return [
false,
`body has ${bullets.length} bullets. Summarise the reason for the change instead.`
]
}
if (pathBullets.length > 6) {
return [
false,
`body lists ${pathBullets.length} changed files. The diff already lists them.`
]
}
return [true, '']
}

module.exports = {
extends: ['@commitlint/config-conventional'],
parserOpts: {
headerPattern: /^(\w*)\((\w*)\)-(\w*)\s(.*)$/,
headerCorrespondence: ['type', 'scope', 'subject']
},
plugins: [
{
rules: {
'body-max-lines': bodyMaxLines,
'body-no-changelog': bodyNoChangelog
}
}
],
// https://commitlint.js.org/reference/rules.html
rules: {
// The header is unbounded because multi-package scopes are long, e.g.
// `fix(ui-drawer-layout,ui-a11y-utils):`. The subject itself is capped.
'header-max-length': [0, 'always', 150], // 0 === rule is disabled
'subject-max-length': [2, 'always', 150]
'subject-max-length': [2, 'always', 72],
'body-max-line-length': [2, 'always', 100],
'body-max-lines': [2, 'always', 28],
'body-no-changelog': [2, 'always']
},

// https://cz-git.qbb.sh/config/
Expand Down
12 changes: 12 additions & 0 deletions docs/contributing/contributing-getting-started.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,18 @@ Please update the documentation and examples with any changes.
- Write documentation inline in code comment blocks. The code and docs should
always be in sync.

### Code Comments

Write comments for someone reading the file a year from now, with no access to
the pull request or ticket that introduced the change.

- Explain _why_, not _what_. If the code already says it, leave the comment out.
- Keep it to one line where you can.
- Don't refer to the change itself ("we now…", "previously…", "this fix…") or
narrate the diff ("added onKeyDown handler") - `git log` covers that.
- Document props with a JSDoc block containing one prose sentence. Types are
parsed from TypeScript, so `@param` and `@type` tags aren't needed.

### Commit Guidelines

Run `git commit` to commit your changes and follow our commit message format.
Expand Down
Loading
Loading