Crafting Quality Code with Conventional Comments

Most code review friction is not technical. It is ambiguity. “Maybe rename this?” leaves the author guessing whether it blocks the merge. Conventional Comments 🔗 fixes that with a one-word prefix that tells the author exactly what to do with your feedback.

The format is:

<label> [(decorations)]: <subject>

[discussion]

That is the whole spec. The rest of this post is how to apply it without it feeling like ceremony.

The labels, in order of how often you should use them

If you find yourself reaching for a label that is not on this list, you do not need a new label. You need to pick one of these.

Decorators: the part most teams skip

Decorators sit in parentheses after the label and answer “how seriously?” They are the single biggest upgrade over plain comments.

• (blocking): must be resolved before merge.
• (non-blocking): do not let this hold up the PR.
• (if-minor): address only if the fix is small.

Default conventions worth adopting team-wide:

• nitpick, thought, praise, question are non-blocking.
• issue, todo, chore are blocking.
• suggestion is ambiguous, so always decorate it.

That last point is the rule that pays for itself. A bare suggestion: is the comment authors stall on most.

Copy-paste examples that work in real PRs

Suggestion, non-blocking

suggestion (non-blocking): Pull the retry logic into `withRetry()` so we can
reuse it in `syncProjects.ts`. Fine to defer to a follow-up PR.

The author can merge today and open a ticket. No standoff.

Suggestion, blocking, with a reason

suggestion (blocking): Use `AbortController` here instead of a boolean flag.
The current code keeps the fetch running after unmount and writes to state,
which will throw in StrictMode.

“Blocking” plus the why. The author knows what to change and why arguing is not worth it.

Issue with a reproduction

issue (blocking): `parseAmount("1,000.50")` returns `1`. The regex strips
commas before the decimal check. Repro:

  expect(parseAmount("1,000.50")).toBe(1000.5); // fails, returns 1

A failing test in the comment is worth ten paragraphs of prose.

Question that is actually a hidden issue

question: Is `userId` guaranteed to be set when this runs? The caller in
`onSubmit` reads it from `session`, which can be `null` during sign-out.

If the answer is “no,” it converts to an issue. If the answer is “yes, because X,” that X probably belongs in a comment in the code.

Nitpick the author can ignore guilt-free

nitpick: `useUserData` reads slightly better than `useUserDataHook` to me.
Take it or leave it.

The “take it or leave it” is the point. Without it, every nitpick becomes a small negotiation.

Praise that is specific

praise: The `Result<T, E>` wrapper here cleaned up four call sites that used
to swallow errors. Nice catch.

Generic praise (“nice!”) is noise. Specific praise teaches the rest of the team what good looks like.

Thought that is not a request

thought (non-blocking): If we end up with a third permission check like
this, a `usePermission(action, resource)` hook might be worth it. Not for
this PR.

Labels the idea as a seed, not a demand. Future readers searching the repo for “permission” will find your reasoning.

Patterns that make reviews faster

Lead with the strongest blocker

If you have one issue (blocking) and twelve nitpicks, post the issue first and on its own. A wall of nitpicks buries the bug.

Convert “I would…” into a concrete diff

Instead of:

suggestion: I would extract this.

Write:

suggestion (if-minor): Extract to a helper:

  function normalizeEmail(input: string) {
    return input.trim().toLowerCase();
  }

Suggestions with code attached get applied. Suggestions without code get debated.

Use question before issue when you are not sure

Asking “why does this set isLoading after the fetch resolves?” is cheaper than asserting it is wrong and being wrong yourself. If the author confirms it is a bug, edit the comment to issue (blocking) so the thread reads correctly later.

Resolve threads with the label that ended them

When you fix something based on a suggestion, reply with done or push the commit and resolve. When you push back, say so: disagree: keeping it inline because the helper is only used here. The thread stays scannable.

Anti-patterns to drop today

• Bare prefixes without a verb. issue: this is broken is a hand-wave. State what is broken and what to do.
• Stacking labels. suggestion/issue: means you have not decided. Decide first.
• Using nitpick for things that are not nitpicks. If it would block your own PR, it is not a nitpick. Calling it one trains the team to ignore real feedback.
• Reviewing line-by-line when the design is wrong. If the approach is off, post one top-level issue (blocking) describing the architectural concern. Do not annotate fifty lines that should not exist.
• Praise inflation. Praising every other line dilutes the signal. Save it for the bits you would actually copy into your own code.

Rolling it out without a launch announcement

You do not need team buy-in to start. Use the labels yourself for two weeks. Two things tend to happen: authors start asking what your decorators mean (good, now you explain once), and other reviewers begin mirroring the format because it is faster to read.

If you want to formalize it later:

1. Add a one-page section to your contributing docs with the table above.
2. Set a default in your review tool. GitHub saved replies, GitLab quick actions, and Graphite all support templated comments.
3. Add suggestion (blocking), issue (blocking), and nitpick as canned comments. That is 80% of usage.

That is the whole adoption plan.

TL;DR

Conventional Comments is a prefix and a parenthetical. The prefix tells the author what kind of feedback this is. The parenthetical tells them whether it blocks merge. Use suggestion with a decorator, use nitpick honestly, attach code to suggestions, and put the blocker first. Reviews stop stalling on “what did they mean?” and start moving on “here is the fix.”

Written by Zubin