Skip to content
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

buildNpmPackage: use env vars #280623

Closed

Conversation

Fresheyeball
Copy link
Contributor

Description of changes

Ayyyyyeee, you know it should be env vars not .npmrc that we use to set settings. That way we can use the hook in a nix shell without modifying the user's state. You know?

Things done

I tested this via usage of the hook in a nix shell, worked.

@ursi
Copy link
Contributor

ursi commented Jan 22, 2024

A potentially not good PR I made on top of this one that fixes an issue with HOME Fresheyeball#2

@dotlambda dotlambda changed the title use env vars in buildNpmPackage buildNpmPackage: use env vars Jan 31, 2024
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

LGTM besides the commit message.
I successfully built a few packages that use buildNpmPackage.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Mar 8, 2024
@FliegendeWurst FliegendeWurst added the awaiting_changes (old Marvin label, do not use) label Nov 30, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 2, 2025
@SigmaSquadron SigmaSquadron removed the awaiting_changes (old Marvin label, do not use) label Jan 5, 2025
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 11, 2025
@FliegendeWurst FliegendeWurst changed the base branch from master to staging January 11, 2025 23:46
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 12, 2025
@FliegendeWurst
Copy link
Member

Closing in favor of the rebased #373220

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.

6 participants