-
Notifications
You must be signed in to change notification settings - Fork 65
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: refactor 'nix develop' flake #41
Conversation
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.
Change itself looks fine, but I have two requests for the commit message:
- Switch it from
feat:
torefactor:
- Include the PR description in the body of the commit
I want to make sure we have a commit history that provide enough info for people to fairly easily make sense of our changes in the future.
Force pushing to the branch is fine to update the commit message. |
I probably should expand slightly for clarity: I overall don't care that much about conventional commit style, but I think it makes sense to be somewhat strict about |
9c049cd
to
685805e
Compare
@djwhitt no problem is the commit message ok this way now? |
wait let me remove those leading whitespaces in my commit message.... EDIT: done |
* removed shell.nix * pinned nixpkgs * added flake-utils
685805e
to
bba8fcd
Compare
@hlolli Looks good. Appreciate it! 🙏 Rebased and merged into |
using the legacy nixpkgs caused an issue on my aarch64-darwin system, as it tried to execute linux binaries, therefore I pass the system variable into the import of nixpkgs, and the issue is gone.