8000 fix(minimal): say DNS, min.internal and box IPs plainly in finalize-install --show by norrietaylor · Pull Request #2174 · gominimal/minimal · GitHub
[go: up one dir, main page]

Skip to content

fix(minimal): say DNS, min.internal and box IPs plainly in finalize-install --show - #2174

Merged
norrietaylor merged 2 commits into
release/0.7.0from
fix/forward-port-finalize-install-wording
Oct 12, 2026
Merged

norrietaylor merged 2 commits into
release/0.7.0from
fix/forward-port-finalize-install-wording

Conversation

@norrietaylor
@norrietaylor norrietaylor commented Oct 11, 2026 •
Copy link
Copy Markdown
Member

Forward-port of #2166 to main. That change merged to release/0.7.0 only, so main still prints the old wording.

A clean cherry-pick of 4ac474e with no other change.

Test plan

  • CI lanes on this PR; the change was green on the release branch.

Summary by CodeRabbit

  • Installation
    • Clarified that Minimal provides the DNS service and that DNS names apply to boxes across programs on the machine.
    • Installation summaries now show missing DNS setup details as separate, counted bullet points.
    • macOS setup guidance now notes that box IPs are reserved at every boot.
  • Documentation
    • Updated release-pipeline wording to refer to the DNS service installation step.

@norrietaylor
norrietaylor requested a review from a team as a code owner October 11, 2026 21:28
@coderabbitai
coderabbitai Bot commented Oct 11, 2026 •
Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 18db5f68-f7ac-4eed-b913-4bcf7e1bd894

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: c6f329de-e8ad-4746-bb52-dc87c5e8b9d0



📥 Commits

Reviewing files that changed from t 8000 he base of the PR and between 5cd498c and 0c5b09e.




📒 Files selected for processing (6)
  • crates/minimal/src/cmd/finalize_install.rs
  • crates/minimal/src/resolver.rs
  • docs/internal/release-pipeline.md
  • scripts/install_test.sh
  • scripts/session-e2e.sh
  • scripts/stage-release.sh



Included review availability: This review used your included allowance. Your plan provides up to 5 included reviews per hour; 0 remain after this review.





📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

The changes update resolver advisory wording and fact structure, add list-based facts to names-install summaries, and revise related shell-test expectations and release terminology.

Changes

DNS Advisory and Install Summary

Layer / File(s) Summary
Resolver advisory facts
crates/minimal/src/resolver.rs
Missing facts now use a vector and are joined when rendered. Advisory text describes the DNS service and loopback IPs, with platform-specific wording reflected in tests.
Names-item facts and summary
crates/minimal/src/cmd/finalize_install.rs
Names items accept multiple facts. Summaries display them as counted bullets, while JSON retains joined facts in today. The item label and step descriptions identify the DNS service and box domain.
Related labels and release wording
docs/internal/release-pipeline.md, scripts/install_test.sh, scripts/session-e2e.sh, scripts/stage-release.sh
Test expectations and release text use the updated DNS-names and minzoned terminology.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: 0chroma





Merge Risk: ⚪ Minimal · up to 0c5b0

The updated DNS advisory and install-summary wording presents no identified merge-blocking risk. Run the planned CI checks before merging.

Pre-merge checks | Passed 4 | Inconclusive 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage Inconclusive Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 4 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly identifies the main change: updating finalize-install output to state DNS, min.internal, and box IP details plainly. It uses a concise Conventional Commit format.
Description check Passed The description explains the forward-port, provides the reason for the change, and includes a test plan. It omits the template's explicit Summary, Testing, and Checklist headings, but the required con…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.



Full details: Docstring Coverage

Explanation

Docstring coverage is 76.92% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 26 functions across 4 files. (2 skipped: 1 unsupported, 1 too large.)






✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR




🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR





  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit reads the DNS lines,
Then counts the facts in tidy signs.
Box names find their proper place,
Loopback notes join the install race.
The burrow boots; the labels shine.

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

@minimal-foundry
minimal-foundry Bot commented Oct 11, 2026 •
Copy link
Copy Markdown
Contributor

Approved: no findings at 0c5b09eb8542.

Nothing to do.

Clobber: none.

@minimal-foundry minimal-foundry Bot left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimal-review — approved

Verdict APPROVE — No findings.

@minimal-foundry minimal-foundry Bot added the review:approved minimal-review approved the current head label Oct 11, 2026
@norrietaylor
norrietaylor changed the base branch from main to release/0.7.0 October 12, 2026 00:59
@norrietaylor
norrietaylor enabled auto-merge (squash) October 12, 2026 01:26
@norrietaylor
norrietaylor merged commit e99c137 into release/0.7.0 Oct 12, 2026
7 checks passed
@norrietaylor
norrietaylor deleted the fix/forward-port-finalize-install-wording branch October 12, 2026 01:28
gominimal-aw-bot Bot added a commit that referenced this pull request Oct 12, 2026
…nstall --show (#2166) (#2174)

(cherry picked from commit 4ac474e)

Co-authored-by: gominimal-aw-bot[bot] <281738952+gominimal-aw-bot[bot]@users.noreply.github.com>
gominimal-aw-bot Bot added a commit that referenced this pull request Oct 12, 2026
…nstall --show (#2166) (#2174)

(cherry picked from commit 4ac474e)

Co-authored-by: gominimal-aw-bot[bot] <281738952+gominimal-aw-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review:approved minimal-review approved the current head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

0