-
Notifications
You must be signed in to change notification settings - Fork 15
Fix Nix build & direnv config #70
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
Conversation
Patch is necessary because Nix doesn't believe in testing what's not being distributed (the release version). See (unofficial) https://ryantm.github.io/nixpkgs/languages-frameworks/rust/#tests-relying-on-the-structure-of-the-target-directory. And since Nix supports distributing to multiple architectures, target architecture is always specified.
For when you'd want to test non-debug build binaries
thanks for making that change! could you fix clippy, and then I think we're good to go on this |
Done in fced6a0 =) Feels like I touched a lot of code that clippy complained about that I haven't touched though... |
seems like it's still complaining..? not sure why you had to make so many changes, seems like something is afoot |
Ahhh, I see what happened now. I ran clippy w/some configured defaults, since at first I gave up getting the incantation at https://github.com/x10an14/nufmt/blob/7487a1f5731626f9296b6e2359c3bb8392f36f7d/.github/workflows/pull_request.yml#L26 to work for me locally. I tried again with some more hand-rolling effort, and now I see that my local output matches (I didn't think to check previously, hence the big commit of unrelated clippy fixes, mea culpa) the previous runs' error(s). |
could you reduce the clippy changes to just what's relevant to this PR? I'm not opposed to the clippy changes resulting from your config necessarily but it doesn't seem appropriate to put them in a PR for fixing the Nix build, especially when some of them include function signature changes |
This reverts commit 36c31e8.
Done in 2809ce2. Feel free to revert the revert if y'all want em after all. |
thanks! let's get this merged finally 😄 |
Description of changes
Relevant Issues
None