Skip to content

fix(nix): address PR #778 review comments#800

Merged
cjpais merged 1 commit intocjpais:mainfrom
y0usaf:fix/nix-pr778-review
Feb 13, 2026
Merged

fix(nix): address PR #778 review comments#800
cjpais merged 1 commit intocjpais:mainfrom
y0usaf:fix/nix-pr778-review

Conversation

@y0usaf
Copy link
Copy Markdown
Contributor

@y0usaf y0usaf commented Feb 12, 2026

Before Submitting This PR

Please confirm you have done the following:

Human Written Description

This addresses the review comments from @pinage404 on #778. The Nix build was using patchShebangs + nodejs when it only needed to fix two shebangs (tsc and vite), and was using the full store path for bun. Comments added for explanations in some areas too.

Related Issues/Discussions

Follow-up to #778 (review comments)

Community Feedback

@pinage404 left 5 review comments on #778 that were merged without being addressed. This PR resolves them:

  1. Add comment explaining platform restriction — no longer applicable, PR fix(nix): fix ALSA mismatch, add GStreamer plugins, and repair UI environment #787 added aarch64-linux to supportedSystems
  2. Remove nodejs dependency — replaced patchShebangs with targeted substituteInPlace
  3. Use bun for shebangs instead of nodetsc/vite shebangs now point to bun
  4. Use bun run build instead of ${pkgs.bun}/bin/bun run buildbun is already on PATH
  5. Add comment explaining doCheck = false — documents sandbox limitations

Testing

  • nix build .#handy completes successfully
  • result/bin/handy binary exists and is executable
  • ldd result/bin/handy — all shared libraries resolve
  • nix path-info -r .#handy | grep -i node — returns nothing (nodejs not in closure)

Screenshots/Videos (if applicable)

N/A

AI Assistance

  • No AI was used in this PR
  • AI was used (please describe below)

If AI was used:

  • Tools used: Claude Code
  • How extensively: AI applied the changes to flake.nix and ran the nix build verification. Human directed which review comments to address and reviewed the approach.

- Replace patchShebangs with targeted substituteInPlace for tsc/vite
  shebangs, pointing them to bun instead of node
- Use `bun run build` instead of `${pkgs.bun}/bin/bun run build`
  since bun is already in nativeBuildInputs
- Remove nodejs from nativeBuildInputs (no longer needed)
- Add comment explaining why doCheck is disabled
@cjpais cjpais merged commit 3b88717 into cjpais:main Feb 13, 2026
4 checks passed
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.

3 participants