Skip to content

feat(nix): add mkShell and flake #580

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dtomvan
Copy link

@dtomvan dtomvan commented May 13, 2025

No description provided.

Comment on lines +88 to +101
"{pkgs ? import <nixpkgs> {}, ...}:",
"pkgs.mkShell {",
" shellHook = ''",
" $1",
" '';",
"",
" env = {",
" $2",
" };",
"",
" packages = [",
" $3",
" ];",
"}"
Copy link
Contributor

@Eveeifyeve Eveeifyeve May 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"{pkgs ? import <nixpkgs> {}, ...}:",
"pkgs.mkShell {",
" shellHook = ''",
" $1",
" '';",
"",
" env = {",
" $2",
" };",
"",
" packages = [",
" $3",
" ];",
"}"
"pkgs.mkShell {",
" shellHook = ''",
" $1",
" '';",
"",
" env = {",
" $2",
" };",
"",
" packages = [",
" $3",
" ];",
"};"

for this part I would make it both nix and flake compatible since it would be better for both compatibility and a user is expected to write pkgs.mkShell in a shell.nix file or in devShells.default

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, makes sense, but then I'd need to write the preamble manually every single time I write a shell.nix...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably best to split it into two templates in that case.

"",
" inputs = {",
" nixpkgs.url = \"github:NixOS/nixpkgs/nixos-unstable\";",
" flake-utils.url = \"github:numtide/flake-utils\";",
Copy link
Contributor

@Eveeifyeve Eveeifyeve May 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I would not use flake-utils as it's not module system compliant, but I would say split up to flake-parts and flake-utils as separate snippets and do the default or just remove flake-utils from this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about lib.genAttrs <SYSTEMS> (system: {}) so we'd only need nixpkgs? We could add a different flake-parts snippet but that might get out of hand quite quickly as flake.parts is quite the rabbit hole.

Copy link
Contributor

@Eveeifyeve Eveeifyeve Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say probably best to create one with all systems, flake utils and flake parts.

Copy link
Contributor

@Eveeifyeve Eveeifyeve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say my reviews are helpful suggestion, I can't necessarily approve unless specifically the pkgs.mkShell part is fix for compatibility and/or the flake-utils usage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants