Skip to content

docs(readme): update Homebrew installation to use core formula instead of tap - #821

Merged
colinhacks merged 3 commits into
nubjs:mainfrom
fraluc06:update-brew-installation
Sep 2, 2026
Merged

docs(readme): update Homebrew installation to use core formula instead of tap#821
colinhacks merged 3 commits into
nubjs:mainfrom
fraluc06:update-brew-installation

Conversation

@fraluc06

Copy link
Copy Markdown
Contributor

closes #728

@vercel

vercel Bot commented Aug 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
nub Ready Ready Preview Sep 1, 2026 11:46pm UTC

Request Review

@pullfrog pullfrog 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.

Important

The change itself is correct and verified, but closes #728 on a README-only edit closes the tracking issue while five other user-facing surfaces still point at the tap.

Reviewed changes — a one-line README.md edit swapping the Homebrew install command, plus verification that the underlying claim holds and that nothing else in the toolchain depends on the fully-qualified tap name.

  • Homebrew install linebrew install nubjs/tap/nubbrew install nub in the install block (README.md:65).

Verified independently:

  • nub is in homebrew/core. formulae.brew.sh/api/formula/nub.json returns version 0.8.1, matching this repo's Cargo.toml, so the core formula is current rather than lagging. The bare command is accurate today.
  • The degraded-runtime risk flagged during #728 triage is closed. crates/nub-core/build.rs now hard-fails an embed-runtime build whose runtime/node_modules is missing any of the five vendored deps, so a green core bottle implies they were staged.
  • nub upgrade needs no accompanying change. detect_channel classifies by path (/homebrew/, /Cellar/, /linuxbrew/) and HOMEBREW_UPGRADE_ARGS is the unqualified ["upgrade", "nub"], so a core install upgrades correctly.

⚠️ The README is one of six surfaces, and closes #728 retires the issue that tracks the rest

Five other user-facing files still say brew install nubjs/tap/nub, including the docs site and start.md — the surfaces most users and coding agents actually hit. Merging as-is auto-closes #728, so the remaining sweep loses its tracker and the site keeps pointing at the tap indefinitely. AGENTS.md is explicit that a user-facing change is not done until site/content/docs/ reflects it.

Either extend this PR to the other five, or downgrade the body to Refs #728 so the issue stays open for the sweep.

Technical details
# Complete the Homebrew core docs sweep

## Affected sites
- `site/content/docs/index.mdx:49` — install docs, fenced `brew install nubjs/tap/nub`
- `site/content/docs/faq.mdx:379` — "Is there a curl install script?" answer, inline mention
- `site/content/docs/node.mdx:180``nub node shim` fresh-install story, inline mention
- `site/src/components/install-tabs.tsx:14` — homepage install tabs, the `brew` row's `command`
- `site/public/start.md:48` — the agent-facing skill doc served at `nubjs.com/skill.md`

## Deliberately NOT in scope
- `.claude/skills/release/SKILL.md:266,268` and `scripts/download-stats.mjs:20` keep the
  fully-qualified name. They verify the tap channel, which `release.yml`'s `bump-homebrew-tap`
  job still publishes on every stable release.

## Required outcome
- Every user-facing install instruction names the same command, or the PR body uses `Refs #728`
  instead of `closes #728` so the sweep stays tracked.

ℹ️ Existing tap users get no migration line, and the tap's future is undecided

Anyone already on nubjs/tap/nub cannot simply run brew install nub — the same-named keg is already installed from another tap, so they need to uninstall and untap first. The #728 triage comment proposed exactly this line, and it has no home in the README's terse command block, so it belongs in the docs or the release notes rather than here.

Underneath it sits a scope question only the maintainer can settle: bump-homebrew-tap (.github/workflows/release.yml:2632) still pushes to nubjs/homebrew-tap on every release, so the tap remains the fast channel while core lags behind a bump PR. Whether that is a deliberate dual track or the tap is now deprecated changes how the docs should word this.

Technical details
# Migration guidance for tap → core

## Required outcome
- Existing tap users have a documented path off the tap, e.g.
  `brew uninstall nubjs/tap/nub && brew untap nubjs/tap && brew install nub`.
- The docs state which channel is canonical, consistently with whatever is decided below.

## Open questions for the human
- Is `nubjs/homebrew-tap` a deliberate fast channel (it updates inside the release run, while
  core lags until a bump PR lands), or is it being deprecated now that core carries `nub`?
- Should the release checklist gain a step confirming the core bump landed per release?
  Right now nothing in `.claude/skills/release/SKILL.md` checks core.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

@pullfrog pullfrog 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.

ℹ️ The migration note is correct and I verified it against Homebrew's actual behavior — one structural suggestion inline.

Reviewed changes — the delta since the prior review is the new > [!NOTE] callout in README.md giving existing tap users a migration path, plus the fence restructuring it required.

  • Added a tap → core migration note> [!NOTE] block at README.md:68-72 documenting brew uninstall nubjs/tap/nub && brew untap nubjs/tap && brew install nub.
  • Split the install code block — the single sh fence now closes at line 66 and reopens at line 73 to make room for the note, so Nix / mise / npm land in a second block.

This addresses the migration-guidance gap raised on the previous commit. I verified the command independently rather than taking it on faith, and every step holds:

  • nubjs/homebrew-tap ships no tap_migrations.json, so brew update does not auto-migrate. The manual path is genuinely required.
  • With nubjs/tap/nub installed, a bare brew install nub does not install the core formula — it warns … is already installed, it's just not linked (Homebrew/brew#17941) and the user silently keeps running the tap build. The note prevents a real, quiet failure.
  • Uninstall-before-untap is load-bearing: brew untap refuses a tap with installed formulae unless --force is passed. The ordering is right.
  • No brew link step is missing — the generated formula is not keg_only.

ℹ️ The README now tells users to leave the tap while the docs site still tells them to join it

This is the previous review's sweep finding, but the new note changes its character rather than just leaving it open. site/content/docs/index.mdx:49, site/src/components/install-tabs.tsx:14, and site/public/start.md:48 still hand new users brew install nubjs/tap/nub — so the site keeps enrolling people into the exact state the README's note now exists to rescue them from. Every new install through the site's copy-paste tabs becomes a future migration.

Nothing to change in this diff; flagging it because closes #728 retires the tracker for the remaining five surfaces.

Technical details
# Site surfaces still pointing at the tap

## Affected sites
- `site/src/components/install-tabs.tsx:14` — homepage install tabs, the `brew` row's `command`
- `site/content/docs/index.mdx:49` — install docs, fenced `brew install nubjs/tap/nub`
- `site/public/start.md:48` — the agent-facing skill doc served at `nubjs.com/skill.md`
- `site/content/docs/faq.mdx:379` — "Is there a curl install script?" answer, inline mention
- `site/content/docs/node.mdx:180``nub node shim` fresh-install story, inline mention

## Deliberately NOT in scope
- `site/content/blog/nub-0-7-1.mdx:11` is a historical release note; the tap-qualified command
  was accurate at the time and should stay.
- `.claude/skills/release/SKILL.md:266,268` and `scripts/download-stats.mjs:20` verify the tap
  channel, which `release.yml`'s `bump-homebrew-tap` job still publishes on every stable release.

## Required outcome
- Either the site surfaces name `brew install nub`, or the PR body uses `Refs #728` so the sweep
  stays tracked after merge.

## Open questions for the human
- Is `nubjs/homebrew-tap` a deliberate fast channel (it updates inside the release run, while core
  lags until a bump PR lands), or is it deprecated now that core carries `nub`? The answer decides
  whether the site should recommend core outright or present both.

ℹ️ Nitpicks

  • README.md:72 — the closing > line carries a trailing space and is otherwise empty, which renders as blank padding at the bottom of the alert box. Dropping the line entirely reads better than trimming it.
  • The commit subject has a typo: add istructionsadd instructions. Worth fixing if the branch gets amended for anything else; not worth a rewrite on its own.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread README.md Outdated
…al core tap

Added note about uninstalling previous nub installation from custom tap before Homebrew installation.

@pullfrog pullfrog 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.

ℹ️ Everything raised on the previous commit is fixed. No new issues — one carry-forward that only the maintainer can close out.

Reviewed changes — the branch was force-pushed, so the delta against the prior review is a repositioning of the migration note rather than new content.

  • Moved the > [!NOTE] below the install fence — the note now sits at README.md:77-81, after the single sh block closes at line 75, so Nix / mise / npm are back in one unbroken install list instead of a detached second fence.
  • Cleaned up the alert block — the empty > line that carried trailing whitespace is gone; verified with cat -A that lines 75-81 contain no trailing spaces and that line 79 is a bare >.
  • Fixed the commit subject typoadd istructionsadd instructions.

The Homebrew facts behind the note were verified on the previous run and are unchanged: nub is in homebrew/core at 0.8.1, nubjs/homebrew-tap ships no tap_migrations.json so the manual path is genuinely required, and uninstall-before-untap is the correct ordering.

ℹ️ Five site surfaces still send new users to the tap, and closes #728 will retire their tracker

Nothing to change in this diff — flagging only because the merge closes the issue. Re-grepped at 1a5653c: site/src/components/install-tabs.tsx:14, site/content/docs/index.mdx:49, site/public/start.md:48, site/content/docs/faq.mdx:379, and site/content/docs/node.mdx:180 still print brew install nubjs/tap/nub, so the site keeps enrolling people into the state this PR's note now exists to rescue them from.

This is the third run carrying the point, so treating it as settled either way is fine — it needs a decision, not another fix.

Technical details
# Site surfaces still pointing at the tap

## Affected sites
- `site/src/components/install-tabs.tsx:14` — homepage install tabs, the `brew` row's `command`
- `site/content/docs/index.mdx:49` — install docs, fenced `brew install nubjs/tap/nub`
- `site/public/start.md:48` — the agent-facing skill doc served at `nubjs.com/skill.md`
- `site/content/docs/faq.mdx:379` — "Is there a curl install script?" answer, inline mention
- `site/content/docs/node.mdx:180``nub node shim` fresh-install story, inline mention

## Deliberately NOT in scope
- `site/content/blog/nub-0-7-1.mdx:11` is a historical release note; the tap-qualified command
  was accurate at the time and should stay.
- `.claude/skills/release/SKILL.md:266,268` and `scripts/download-stats.mjs:20` verify the tap
  channel, which `release.yml`'s `bump-homebrew-tap` job still publishes on every stable release.

## Required outcome
- Either the site surfaces name `brew install nub`, or the PR body uses `Refs #728` so the sweep
  stays tracked after merge.

## Open questions for the human
- Is `nubjs/homebrew-tap` a deliberate fast channel (it updates inside the release run, while core
  lags until a bump PR lands), or is it deprecated now that core carries `nub`? The answer decides
  whether the site should recommend core outright or present both, and whether the release
  checklist should gain a step confirming the core bump landed.

Pullfrog  | Fix it ➔View workflow run | Using Claude Opus𝕏

The homebrew-core formula went live in Homebrew/homebrew-core#298614, so
`brew install nub` now resolves without a tap. README was updated already;
this covers the remaining five surfaces a reader actually copies from.

The nubjs/tap tap stays live and the release run keeps bumping it, so an
existing tap install continues to upgrade from the tap.

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes since the prior Pullfrog review covered the final Homebrew documentation sweep and the revised migration callout.

  • Completed the install-surface sweep. Updated the five remaining user-facing Homebrew instructions to use brew install nub, bringing all six surfaces into agreement.
  • Refined the migration callout. Clarified the tap-to-core wording and rendered the migration command as a shell block without splitting the main install list.

Pullfrog  | View workflow run | Using GPT Sol𝕏

@colinhacks
colinhacks merged commit d722e04 into nubjs:main Sep 2, 2026
37 checks passed
@colinhacks

Copy link
Copy Markdown
Contributor

Shipped in v0.8.3: https://github.com/nubjs/nub/releases/tag/v0.8.3 — thanks for the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Submit nub to the official brew core tap

2 participants