-
Notifications
You must be signed in to change notification settings - Fork 46
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
Nix update and cleanups #83
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.
One comment, rest LGTM 👍
flake.nix
Outdated
@@ -45,15 +42,12 @@ | |||
# riscv-arch = "rv32imc"; | |||
# }; | |||
|
|||
pythonEnv = pkgs.python3.withPackages ( |
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.
To me it makes more sense to assemble python environments on a per-project basis, pulling in patched versions of dependencies from lowrisc-nix when needed. I don't really see the point of keeping the env itself in the common repo?
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 have converted the env to use poetry
flake.nix
Outdated
if [ -z "$DIRENV_IN_ENVRC" ]; then | ||
cat <<'EOF' |
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.
Indentation look strange?
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.
Bash heredoc doesn't support indentation
* Removed verilator overlay. It's sufficient to add the dependencies to buildInputs * Remove unneeded comment and code * Use toolchain from lowrisc-nix
Depends on #85