brainstorming
brainstorming
You MUST use this before any creative work - creating features, building components, adding functionality, modifying behavior, or planning anything new, in software or out of it (a talk, a business, a renovation).
engineering-review-commentsUse when drafting, rewriting, classifying, or improving code review comments as a reviewer. TRIGGER on rough review notes, requests to make comments clearer or kinder, severity labels, "how should I phrase this review comment?", "write review comments", author pushback, or turning findings into PR comments. If role is unclear, ask reviewer or author. DO NOT…
Independent GitHub snapshot · Automated classification · Original documentation governs
Review comments should improve the code and the developer's future judgment. They should be clear about severity, grounded in engineering reasoning, and directed at the code rather than the person.
Adapted from Google Engineering Practices Documentation, especially "How to write code review comments" and "Handling pushback in code reviews." Source: https://google.github.io/eng-practices/review/reviewer/comments.html. License: CC-BY 3.0.
Use this structure for non-trivial comments:
SEVERITY: Observation about the code.
Why this matters: engineering reason, user impact, maintainability impact, or violated project rule.
Requested action: concrete fix, question, or decision the author needs to make.
Keep tiny comments tiny. Use the full shape when intent could be misunderstood.
BLOCKER: must change before approval because correctness, maintainability, security, data, test, or operational risk is unacceptable.IMPORTANT: should change before approval; can be waived only with an explicit rationale.QUESTION: reviewer needs information before deciding.NIT: minor polish. Do not block approval.OPTIONAL or CONSIDER: a suggestion the author can decline.FYI: educational context or future consideration; no action expected.Use these patterns directly when helpful:
BLOCKER: This path can return success before the write is durable.
If the process crashes after this point, callers will believe the operation completed even though the data may be missing. Please either move the success response after the durable write or make the operation explicitly asynchronous and observable.
QUESTION: What guarantees that this callback cannot run concurrently?
If it can run in parallel, `pending` can be decremented twice from the same value. If the framework serializes this callback, please add a short comment at the boundary where that guarantee comes from.
NIT: This helper name reads as if it validates the payload, but it only normalizes whitespace. Could we rename it to `normalizePayloadText`?
OPTIONAL: Consider extracting this branch into a named helper. The current version is correct, but a helper would make the three policy cases easier to scan.
When the author disagrees:
Do not keep arguing because you wrote the first comment. The goal is the best code, not defending the initial wording.