Emma Thorpe
51df3473ca
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.
1.8 KiB
name, description, metadata
| name | description | metadata | ||||||
|---|---|---|---|---|---|---|---|---|
| copilot_review_false_positives | Verify Copilot PR-review 'this breaks X' claims against spec/live config before acting; two recorded false positives |
|
The Copilot reviewer on the multicluster / unified-helm repos raises blocking-sounding "this will fail" claims that are sometimes wrong. Verify against the language spec and the live/master config before treating one as real or applying its fix.
Recorded false positives (both Terraform, both Emma-flagged "for future reference"):
- PR #1742 — claimed
var.map.hyphenated-keydot access is parsed as subtraction and breaksterraform plan. False: HCL2 identifiers may contain hyphens (ID_Start (ID_Continue | "-")*), and the same pattern is already live onmasterin prod. Bracket indexing was adopted anyway as marginally clearer, not as a fix. - PR #1745 — claimed the
aks_poolsper-poolmax_surgelookup was off-by-one and should usecount.index + 1. False: every config attribute on that resource indexes withcount.index; only the cosmeticname/az_nodepoollabel uses+1. Applying+1would have introduced a real bug (wrong pool, and out-of-bounds on the last pool).
Why: Blindly applying a plausible-but-wrong Copilot suggestion can introduce a real defect or waste review cycles.
How to apply: For any Copilot claim that code is broken or unsafe, confirm it against the relevant spec and the existing working config first; if it's wrong, say so plainly on the PR and leave the code. Genuine Copilot catches (over-broad WAF @beginsWith, missing input validation, doc/behaviour drift) still get fixed. Relates to workflow-review-and-comments and code_comment_style.