-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
feat: nix direnv #1367
feat: nix direnv #1367
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThis update enhances the development environment by introducing a Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- .envrc (1 hunks)
- .gitignore (1 hunks)
- .vscode/extensions.json (1 hunks)
- .vscode/settings.json (1 hunks)
- flake.nix (1 hunks)
Files skipped from review due to trivial changes (4)
- .envrc
- .gitignore
- .vscode/extensions.json
- .vscode/settings.json
Additional comments not posted (2)
flake.nix (2)
26-26
: LGTM! Addition ofpkgs.direnv
is appropriate.The inclusion of
direnv
in thebuildInputs
list enhances the development environment by automatically managing environment variables based on the directory context.
Line range hint
30-34
:
LGTM!shellHook
configuration is correct.The
shellHook
sets the prompt and sources a.env
file if it exists, which is a standard practice for setting up the development environment.
@@ -23,6 +23,7 @@ | |||
pkgs.nodejs_22 | |||
pkgs.nodePackages.pnpm | |||
pkgs.bun | |||
pkgs.direnv |
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.
Question: I haven't used direnv, but my understanding is that you need it installed outside of the flake so that it knows to activate the flake in the first place? If so, does it make sense to also include it as a dep inside the flake?
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.
I'm not fully sure @shazow. I've always had it installed globally.
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.
IMO it shouldn't be in here, it's kinda like including nix
in here. :P
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.
Yea agreed once you are in nix this becomes useless since you are already in mix
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.
IMO it shouldn't be in here, it's kinda like including
nix
in here. :P
this is a typical usage of direnv
in nix. What do you mean like including nix
?
One use case of direnv
is you don't have to run nix develop
if you use it. Also, your globally installed shell utilities remain available.
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.
That's what I mean, direnv
already needs to be installed for this flake to be loaded, so it's moot to have the flake install it again? Similar to have nix
already needs to be installed for this flake to be loaded.
Unless I don't understand how direnv works correctly. :)
Anyway it's not hurting anything, I just don't think it's helping anything either.
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.
yes good point. What I need to do is add instructions to getting started
# for direnv usage
nix shell nixpkgs#direnv
# then
direnv allow
Description
run
direnv allow
and all the nix packages inflake.nix
will be injected into your shellTesting
Explain the quality checks that have been done on the code changes
Additional Information
Your ENS/address:
esm.eth
Summary by CodeRabbit
New Features
Direnv
for improved environment variable management..envrc
with syntax highlighting in Visual Studio Code.direnv
to package dependencies for streamlined development setups.Bug Fixes
.gitignore
to exclude.direnv
files, improving project cleanliness.Documentation