Emma Thorpe
51df3473ca
CI / flake (pull_request) Successful in 58s
Add two memories and correct one existing, from a review of PR comments across multicluster and unified-helm over the past two months: - code_comment_style: no Jira/ticket IDs in code comments by default, keep comments concise and about the non-obvious why, and use # (not Helm template) comments where they must reach the rendered manifest. - copilot_review_false_positives: verify Copilot blocking claims against the spec and live config before acting; records two Terraform FPs. - workflow_review_and_comments: drop the now-contradicted 'one-liner + WSP ticket reference' guidance, which reviewers repeatedly strip.
2.0 KiB
2.0 KiB
name, description, metadata
| name | description | metadata | ||||||
|---|---|---|---|---|---|---|---|---|
| code_comment_style | Code/comment style from PR review feedback: no ticket IDs in comments by default, concise, explain the non-obvious why |
|
Recurring PR-review feedback from human reviewers (Tom Wilkins, Andrew Hyde, Gilberto Pestanarosa) on the multicluster and unified-helm repos, on how to write comments in code and IaC:
- No Jira/WSP ticket IDs in code comments or WAF
msg:strings by default. Add a ticket ref only when there is a specific reason to. Never duplicate the id, and never put a ticket URL in a comment. Tracking/rationale belongs in the PR description and the Jira ticket, not in.tf,.tftpl, or.yaml. (Flagged repeatedly — PRs #1735, #1762.) - Comment the non-obvious "why", not the obvious "what". Drop comments that restate what the code or file plainly does (e.g. a header on
namespace.yamlre-announcing that it defines a namespace). If a reviewer can't tell why a comment exists, it shouldn't. - Keep it short and readable. No multi-line block where one line does; if a comment isn't clear after a couple of reads, rewrite it plainer. Prefer trimming to the single load-bearing sentence over hedged prose. (PRs #1745, #216.)
- In Helm charts, use
#YAML comments — not{{/* */}}— for anything that must appear in the rendered manifest. Helm template comments are stripped before render, so port/label explanations meant for the live chart have to be#. (PR #216.)
Why: Multiple human reviewers, across multiple PRs, consistently push back on verbose comments and gratuitous ticket references. Terse, purpose-driven comments clear review faster.
How to apply: When writing or editing comments in code/IaC, default to: no ticket id, one line, non-obvious "why" only. This supersedes the "one-liner + WSP ticket reference" phrasing that used to live in workflow-review-and-comments. Relates to docs_keep_updated.