b716aed914
ci: trigger review-gate on self-review comments; run heavy review on issue_comment when reason=='self-review'
a129f062a2
ci: require reason=='self-review' before running heavy review (disable TTL-triggered heavy reviews)
Self-Review: PR #152
Self-review against eb0ff3aa69f152dd995de91c88227d3e32ac2917
Phase 1: Independent Findings
None — diff looks clean.
Reviewed validatedocmap.go and `validatedocmap_…
0c6f46d279
ci: gate review job on current self-review (with Doc consistency); TTL fallback via review-gate job
Self-Review: PR #152
Self-review against eb0ff3aa69f152dd995de91c88227d3e32ac2917
Phase 1: Independent Findings
None — diff looks clean.
Reviewed: validateDocmapPath signature…
Finding #1 (ACK-NOT-VALID): Acknowledged. The reviewer explicitly notes this is a known limitation of the os.SameFile pattern and that "no action is required" — the code comment already labels it defense-in-depth. No change needed.
Finding #4 (ACK-NOT-VALID): os.Lstat is intentionally used here for consistency with checkStaleDocs, which also uses Lstat to avoid implicit symlink-follow semantics. Switching to Stat post-EvalSymlinks would be equivalent in practice but inconsistent with the rest of the file. Deliberate choice — no change needed.
Finding #5 (ACK-NOT-VALID): User-facing error messages intentionally reference *docmapFlag (the original --docmap value) rather than the resolved path. Showing the resolved path would confuse users who specified a symlink — they would see a path they did not specify. Accepted UX tradeoff.
Finding #2 (ACK-NOT-VALID): Acknowledged. The reviewer confirms no issue — the ModeSymlink check removal is correct (the check was genuinely unreachable after filepath.EvalSymlinks), the comment is accurate, and this is a deliberate documented choice. No action needed.
Finding #1 (ACK-NOT-VALID): Acknowledged. The reviewer explicitly states this is a known limitation of the os.SameFile pattern and that "no action is required" — the code comment already labels it defense-in-depth. No code change needed.
Finding #1 (ACK-NOT-VALID): Acknowledged. The reviewer explicitly notes this is a known limitation of os.SameFile and that "no action is required" — the code comment already labels it defense-in-depth. No change needed.
Finding #5 (ACK-NOT-VALID): User-facing error messages intentionally reference *docmapFlag (the original --docmap value) rather than the resolved path. Showing the resolved path in errors would confuse users who specified a symlink — they want to see the path they actually provided.
Finding #4 (ACK-NOT-VALID): os.Lstat is intentionally used here for consistency with checkStaleDocs, which also uses Lstat to avoid implicit symlink-follow semantics. Switching to Stat post-EvalSymlinks would be equivalent but would create an inconsistency within the file.
Finding #2 (ACK-NOT-VALID): Acknowledged. The reviewer confirms no issue — the ModeSymlink removal is correct (the check was genuinely unreachable after EvalSymlinks), the comment is accurate, and the removal is a deliberate documented choice.
Finding #1 (ACK-NOT-VALID): Acknowledged. The reviewer explicitly notes this is a known limitation of the os.SameFile pattern and that "no action is required" — the code comment already labels it defense-in-depth. No code change needed.
Finding #5 (ACK-NOT-VALID): User-facing error messages intentionally reference *docmapFlag (the original --docmap value) for user clarity — showing a resolved path the user never specified would be surprising. The reviewer notes this is acceptable. No change needed.