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
Init devenv, gitignore .nlsp-settings #1942
Conversation
ea7bab6
to
a12737b
Compare
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.
uh i don't know how to review this, @shazow can you maybe take a look?
I haven't used direnv yet (just straight up nix) but I should try it! Will give it a try soon. Looks fairly straightforward. |
I can rework this on plain nix flakes which can also easily be integrated with direnv via |
@onsails One of the nice things about having a flake is that automatically lets you use it as a dependency in another flake, such as for packaging foundry. I assume just having the direnv settings wouldn't be enough for that? |
Please see updated diff. Two birds one stone via. |
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.
A few optional nits, but generally looks great to me. Appreciate that it works with plain nix develop
as well as direnv! :)
Confirmed everything works and builds with:
$ nix develop
pre-commit-hooks.nix: updating /home/shazow/local/src/github.com/gakonst/ethers-rs repo
pre-commit installed at .git/hooks/pre-commit
(devenv) $ cargo build
...
Finished dev [unoptimized + debuginfo] target(s) in 3m 06s
In the future might want to also add a packages
output for using the flake as a dependency inside foundry, but this is great for now. Definitely makes my workflow easier.
@gakonst what do you think?
@@ -2,3 +2,7 @@ | |||
.vscode | |||
/.envrc | |||
.idea | |||
.nlsp-settings |
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.
More up to @gakonst, but personally I just do /.*
on my .gitignores and force-add whatever needs to be there (like .gitignore 😅)
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.
Fine with me.as is!
flake.nix
Outdated
# nightly fmt is required here: | ||
# https://github.com/gakonst/ethers-rs/blob/master/.github/workflows/ci.yml#L123 | ||
# while devenv doesn't yet support customizing it: | ||
# https://github.com/cachix/devenv/issues/211 |
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.
Thank you for adding these comments, really useful!
Also added .gitattributes to hide Cargo.lock and flake.lock diffs |
another force-push, enabled nightly rutstfmt and clippy pre-commit hooks via cachix/devenv#228 (have to wait until it's merged) |
Now it's merged and both rustfmt and clippy commit hooks work with nightly toolchain! |
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.
Nice, looking good.
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.
Defer to you guys on this, merging and hopefully addresses all your issues! Thanks for the patience.
Motivation
Nix is widely used in both rust and blockchain development as it enables reproducible builds which is crucial for dapps security and stability. This PR enables direnv setup for ethers-rs – it's a developer-friendly nix flakes wrapper. If accepted, this feature makes ethers-rs more accessible by contributions who rely on nix in their development cycle.
In a separate commit there is a git ignore of
.nlsp-settings
which is a directory for project-local neovim lsp settings.Solution
PR Checklist