Skip to content

docs(contributor): address PR #1143 review feedback#1145

Merged
mchmarny merged 1 commit into
mainfrom
docs/contributor-review-fixes
Jun 2, 2026
Merged

docs(contributor): address PR #1143 review feedback#1145
mchmarny merged 1 commit into
mainfrom
docs/contributor-review-fixes

Conversation

@mchmarny

@mchmarny mchmarny commented Jun 2, 2026

Copy link
Copy Markdown
Member

Summary

Five follow-up fixes against @pdmack's review of #1143 (already merged):

# File Fix
1 docs/contributor/maintaining.md:62 Broken anchor: recipe.md#bom-regeneration-rulerecipe.md#bom-regeneration (heading has no `-rule` suffix)
2 docs/contributor/validator.md Add a note that `readiness` is a `ValidationConfig` field evaluated inline by `checkReadiness`, not a container-per-validator phase
3 docs/contributor/index.md package table Add `pkg/recipe/oskind` row (it's imported by pkg/recipe, pkg/collector, pkg/snapshotter, and the CLI)
4 docs/contributor/recipe.md merge-precedence arrow Annotate with `(lowest priority)` / `(highest priority)` and note that the arrow is priority, not temporal order
5 docs/contributor/api-server.md Drop fragile line-range links (`middleware.go#L45-L61`, `errors.go#L127`) — they go stale silently on file edits

Motivation/Context

Follow-up to the contributor docs rewrite (#1143). The five items above were raised in review but the PR had already been merged. This PR resolves the residual feedback.

Fixes: #1143 (review follow-up)

Type of Change

Documentation only.

Components Affected

  • `area/docs` — five files in `docs/contributor/`

Testing

  • `make lint` green
  • Anchor target verified by inspecting `recipe.md` heading (`## BOM Regeneration` on line 416)
  • All relative links and anchors validated

Risk Assessment

Low. Doc-only.

Checklist

  • `make lint` passes locally
  • Anchors verified
  • Commit signed

Five fixes against @pdmack's review of the contributor docs rewrite
(#1143 (review)):

- **Fix broken anchor** in maintaining.md → recipe.md#bom-regeneration
  (the heading is `## BOM Regeneration`, no `-rule` suffix).
- **Document readiness phase** in validator.md. The three
  container-per-validator phases (deployment, performance,
  conformance) were already clear, but `readiness` is a first-class
  ValidationConfig field that uses inline constraint evaluation in
  `checkReadiness`, not a phase container. Added a note so
  contributors don't go looking for it under the phase containers.
- **Add `pkg/recipe/oskind`** to the index.md package table.
  Imported by pkg/recipe, pkg/collector, pkg/snapshotter, and the
  CLI to break a cycle that previously forced OS string constants
  to be duplicated.
- **Clarify merge precedence arrow** in recipe.md. Annotated the
  arrow with `(lowest priority)` / `(highest priority)` markers and
  added a one-line note that the arrow is priority, not temporal
  order — the narrative already said this, but the diagram
  benefits from the inline hint.
- **Drop fragile line-range links** in api-server.md
  (`middleware.go#L45-L61`, `errors.go#L127`). Both reference
  source files that get edited frequently and the line range goes
  stale silently. File-level links are sufficient; the file is
  small enough that readers can navigate.

`make lint` green.
@mchmarny mchmarny requested a review from a team as a code owner June 2, 2026 17:03
@mchmarny mchmarny self-assigned this Jun 2, 2026
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 726c5833-dc81-47d1-a710-b661cda4b683

📥 Commits

Reviewing files that changed from the base of the PR and between a57aca8 and 2ebbe0f.

📒 Files selected for processing (5)
  • docs/contributor/api-server.md
  • docs/contributor/index.md
  • docs/contributor/maintaining.md
  • docs/contributor/recipe.md
  • docs/contributor/validator.md

📝 Walkthrough

Walkthrough

This PR makes six targeted improvements to contributor documentation files. It adds a new entry for pkg/recipe/oskind to the package index, removes line-number anchors from two documentation reference links in api-server.md, corrects a BOM regeneration anchor link in the review checklist, clarifies that the merge algorithm diagram shows priority ordering rather than temporal sequence, and explains that the readiness field in ValidationConfig is evaluated via inline constraints in checkReadiness rather than as a container-per-validator phase.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • NVIDIA/aicr#1143: Major rewrite and consolidation of the contributor guide that these changes refine with targeted clarifications and link corrections.

Suggested labels

area/docs

Suggested reviewers

  • lalitadithya
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the purpose of the PR: addressing review feedback from PR #1143.
Description check ✅ Passed The description provides detailed context with a comprehensive table of five specific documentation fixes and their rationale, all related to the changeset.
Linked Issues check ✅ Passed All five changes directly address residual feedback from #1143's review: fixing broken anchors, clarifying ValidationConfig behavior, adding missing pkg/recipe/oskind to index, annotating merge-precedence priority, and removing fragile line-range links.
Out of Scope Changes check ✅ Passed All five changes are tightly scoped follow-up fixes to #1143 and remain within the linked issue's scope of improving contributor documentation accuracy and clarity.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/contributor-review-fixes

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Coverage Report ✅

Metric Value
Coverage 76.3%
Threshold 75%
Status Pass
Coverage Badge
![Coverage](https://img.shields.io/badge/coverage-76.3%25-green)

No Go source files changed in this PR.

@mchmarny mchmarny enabled auto-merge (squash) June 2, 2026 17:25
@mchmarny mchmarny merged commit 3b0750e into main Jun 2, 2026
35 of 36 checks passed
@mchmarny mchmarny deleted the docs/contributor-review-fixes branch June 2, 2026 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants