-
Notifications
You must be signed in to change notification settings - Fork 100
Update wording around upstream Nix #1673
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughRevised user-facing copy in src/cli/subcommand/install/determinate.rs: updated Determinate Nix prompt and explainer text, added specific repo/date in timeline, reworded defaulting/flag/support messages, standardized domain references; no functional logic or public API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/cli/subcommand/install/determinate.rs (4)
21-22: Verify product claims and naming for “Determinate Nixd” and FlakeHub auth.Please confirm that “configuration and garbage collection automation” and “easy authentication with FlakeHub” accurately describe current, generally available capabilities and that “Determinate Nixd” is the correct product name used in docs.
If helpful, I can update the phrasing once you confirm scope from the docs/changelog.
63-65: Add the year to avoid ambiguity.Recommend “November 10, 2025” for clarity and future-proofing.
Apply this minimal diff:
- "* {}: the installer will begin to default to Determinate Nix.", - "November 10".bold() + "* {}: the installer will begin to default to Determinate Nix.", + "November 10, 2025".bold()
71-73: Add the year here as well.Match the explicit date style used above.
Apply this minimal diff:
- "* {}: installing upstream Nix will no longer be supported.", - "January 1".bold() + "* {}: installing upstream Nix will no longer be supported.", + "January 1, 2026".bold()
75-77: Prefer a clear deprecation/diagnostic over “no effect.”Consider updating CLI handling so that passing the flag after the cutoff emits a strong deprecation error or warning, rather than silently doing nothing; then mirror that here (e.g., “the flag will be rejected with an error after January 1, 2026”).
If you want, I can prep a small patch to gate the flag post‑cutoff and wire a helpful error to keep users unblocked.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/cli/subcommand/install/determinate.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: grahamc
PR: DeterminateSystems/nix-installer#1664
File: src/settings.rs:56-67
Timestamp: 2025-09-10T15:49:03.281Z
Learning: In the nix-installer codebase, the prefer_upstream CLI flag is intentionally designed as a presence-only flag. Users wanting Determinate Nix should use --determinate instead of --prefer-upstream=false, as this was a deliberate UX design choice to keep the interface simpler.
📚 Learning: 2025-09-10T15:49:03.281Z
Learnt from: grahamc
PR: DeterminateSystems/nix-installer#1664
File: src/settings.rs:56-67
Timestamp: 2025-09-10T15:49:03.281Z
Learning: In the nix-installer codebase, the prefer_upstream CLI flag is intentionally designed as a presence-only flag. Users wanting Determinate Nix should use --determinate instead of --prefer-upstream=false, as this was a deliberate UX design choice to keep the interface simpler.
Applied to files:
src/cli/subcommand/install/determinate.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-aarch64-darwin / Build aarch64 Darwin (static)
- GitHub Check: build-aarch64-linux / Build aarch64 Linux (static)
- GitHub Check: build-x86_64-linux / Build x86_64 Linux (static)
- GitHub Check: build-x86_64-darwin / Build x86_64 Darwin (static)
- GitHub Check: Lints
🔇 Additional comments (2)
src/cli/subcommand/install/determinate.rs (2)
14-16: Copy update looks fine.Wording is clear and consistent with the surrounding prompt. No issues.
55-58: LGTM: clear cutoff statement with absolute date.The repo reference and date are explicit and helpful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/cli/subcommand/install/determinate.rs (1)
64-70: Flag wording + date specificity.
- Presence‑only “prefer upstream” UX: saying “as the default” suggests persistence. Suggest “for this run.”
- Add the year to avoid ambiguity.
- * {}: the installer will begin to default to Determinate Nix. - After this date, you'll need to apply the `{}` flag to install upstream Nix as the default. - "November 10".bold() - "--prefer-upstream-nix".italic() + * {}: the installer will begin to default to Determinate Nix. + After this date, use the `{}` flag to install upstream Nix for this run. + "November 10, 2025".bold() + "--prefer-upstream-nix".italic()Confirm the canonical flag name(s) defined in clap before we ship this copy:
#!/bin/bash # Verify flag names/usages rg -nP -C2 '(?i)#\s*\[\s*clap' --type=rust | rg -nP -C2 '(prefer[-_ ]upstream|determinate)' rg -nP --type=rust '(--prefer-upstream-nix|--prefer-upstream\b|--determinate\b)'
🧹 Nitpick comments (3)
src/cli/subcommand/install/determinate.rs (3)
14-14: Minor copy tweak: hyphenate “developer‑experience”.
Improves readability without changing meaning.-It offers stable flakes, lazy trees, parallel evaluation, a wide variety of developer experience improvements, and more. +It offers stable flakes, lazy trees, parallel evaluation, a wide variety of developer‑experience improvements, and more.
16-16: Tighten phrasing.
Slightly clearer sentence.-Selecting 'no' installs upstream Nix, which comes from nixos.org.\ +Selecting 'no' installs upstream Nix from nixos.org.\
72-78: Add year for “January 1” and keep wording consistent.
Prevents confusion across years.-* {}: installing upstream Nix will no longer be supported. - The `{}` flag will no longer have any effect. -"January 1".bold() +* {}: installing upstream Nix will no longer be supported. + The `{}` flag will no longer have any effect. +"January 1, 2026".bold() "--prefer-upstream-nix".italic()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/cli/subcommand/install/determinate.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: grahamc
PR: DeterminateSystems/nix-installer#1664
File: src/settings.rs:56-67
Timestamp: 2025-09-10T15:49:03.281Z
Learning: In the nix-installer codebase, the prefer_upstream CLI flag is intentionally designed as a presence-only flag. Users wanting Determinate Nix should use --determinate instead of --prefer-upstream=false, as this was a deliberate UX design choice to keep the interface simpler.
📚 Learning: 2025-09-10T15:49:03.281Z
Learnt from: grahamc
PR: DeterminateSystems/nix-installer#1664
File: src/settings.rs:56-67
Timestamp: 2025-09-10T15:49:03.281Z
Learning: In the nix-installer codebase, the prefer_upstream CLI flag is intentionally designed as a presence-only flag. Users wanting Determinate Nix should use --determinate instead of --prefer-upstream=false, as this was a deliberate UX design choice to keep the interface simpler.
Applied to files:
src/cli/subcommand/install/determinate.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-aarch64-darwin / Build aarch64 Darwin (static)
- GitHub Check: build-aarch64-linux / Build aarch64 Linux (static)
- GitHub Check: Lints
- GitHub Check: build-x86_64-linux / Build x86_64 Linux (static)
- GitHub Check: build-x86_64-darwin / Build x86_64 Darwin (static)
🔇 Additional comments (2)
src/cli/subcommand/install/determinate.rs (2)
81-83: LGTM on the GitHub Action note.
Clear and accurate reference.
21-22: Verified — "Determinate Nixd" name & FlakeHub auth claim are correct.Determinate Nix bundles a daemon called "Determinate Nixd" which exposes CLI auth commands (e.g., determinate-nixd login / status / auth bind) to authenticate with FlakeHub; no change required to src/cli/subcommand/install/determinate.rs lines 21–22.
Description
The wording remains a bit less clear than we'd like it to be. This will hopefully clear away some underbrush. New text:
Checklist
cargo fmtnix buildnix flake checkValidating with
install.determinate.systemsIf a maintainer has added the
upload to s3label to this PR, it will become available for installation viainstall.determinate.systems:Summary by CodeRabbit