feat: C template #27

Merged
Sigmanificient merged 7 commits from c into main 2024-05-14 20:24:13 +00:00
Showing only changes of commit e2607d77cc - Show all commits

View file

@ -19,13 +19,12 @@
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
] (system: function nixpkgs.legacyPackages.${system}); ] (system: function nixpkgs.legacyPackages.${system});
in in
isabelroses commented 2024-05-06 22:16:27 +00:00 (Migrated from github.com)
Review

Can we move the package defection to a file called default.nix

Can we move the package defection to a file called default.nix
Sigmanificient commented 2024-05-06 22:20:00 +00:00 (Migrated from github.com)
Review

Can we move the package defection to a file called default.nix

Is there a need to split the flake? it is quite small

> Can we move the package defection to a file called default.nix Is there a need to split the flake? it is quite small
isabelroses commented 2024-05-06 22:36:00 +00:00 (Migrated from github.com)
Review

It allows it to have old nix compatibility. that’s the main thing for me. Also prevents redeclaring the package a few times.

It allows it to have old nix compatibility. that’s the main thing for me. Also prevents redeclaring the package a few times.
rec { rec {
devShells.default = forAllSystems ( devShells = forAllSystems (pkgs: {
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
pkgs: default = pkgs.mkShell {
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
pkgs.mkShell {
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
hardeningDisable = [ "fortify" ]; hardeningDisable = [ "fortify" ];
isabelroses commented 2024-05-06 22:16:50 +00:00 (Migrated from github.com)
Review

 overlays.default = final: prev: {
   hello = final.callPackage ./default.nix {};
 };

```suggestion overlays.default = final: prev: { hello = final.callPackage ./default.nix {}; }; ```
Sigmanificient commented 2024-05-06 22:20:26 +00:00 (Migrated from github.com)
Review

What does this do?

What does this do?
isabelroses commented 2024-05-06 22:37:19 +00:00 (Migrated from github.com)
Review

this allows the user to consume the overlay and it will be added the nixpkgs list. so i could use enviroment.systemPackages = [ pkgs.hello ]; instead.

this allows the user to consume the overlay and it will be added the nixpkgs list. so i could use `enviroment.systemPackages = [ pkgs.hello ];` instead.
Sigmanificient commented 2024-05-06 22:38:42 +00:00 (Migrated from github.com)
Review

doesn't this cause conflict with the nixpkgs's hello pkgs?

doesn't this cause conflict with the nixpkgs's [hello](https://github.com/NixOS/nixpkgs/blob/nixos-23.11/pkgs/by-name/he/hello/package.nix#L34) pkgs?
inputsFrom = pkgs.lib.attrsets.attrValues packages; inputsFrom = pkgs.lib.attrsets.attrValues packages;
} };
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
); });
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
packages = forAllSystems (pkgs: rec { packages = forAllSystems (pkgs: rec {
default = hello; default = hello;
@ -43,9 +42,12 @@
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
}; };
}); });
apps = rec { apps = forAllSystems (pkgs: rec {
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
default = hello; default = hello;
hello = builtins.mapAttrs (name: value: "${value.hello}/bin/hello") packages; hello = {
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
}; program = "${packages.${pkgs.system}.hello}/bin/hello";
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
type = "app";
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
};
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
});
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
}; };
} }

isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`
isabelroses commented 2024-05-06 22:13:05 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

        hello = pkgs.stdenv.mkDerivation {
          pname = "hello";

          src = ./.;
          nativeBuildInputs = [ pkgs.gnumake ];

          enableParallelBuilding = true;
          V = 1;
          installPhase = ''
            install -D hello $out/bin/hello --mode 0755
          '';
I would prefer to not have rec, also i believe it is pname, not name ```suggestion hello = pkgs.stdenv.mkDerivation { pname = "hello"; src = ./.; nativeBuildInputs = [ pkgs.gnumake ]; enableParallelBuilding = true; V = 1; installPhase = '' install -D hello $out/bin/hello --mode 0755 ''; ```
isabelroses commented 2024-05-06 22:16:46 +00:00 (Migrated from github.com)
Review
        hello = pkgs.callPackage ./default.nix {};
```suggestion hello = pkgs.callPackage ./default.nix {}; ```
TheCodedProf commented 2024-05-06 22:15:23 +00:00 (Migrated from github.com)
Review

Please change the description to something more related to C

Please change the description to something more related to C
Sigmanificient commented 2024-05-06 22:19:31 +00:00 (Migrated from github.com)
Review

I would prefer to not have rec, also i believe it is pname, not name

for mkDerivation it's name: otherwise it would throw error: attribute 'name' missing

> I would prefer to not have rec, also i believe it is pname, not name for `mkDerivation` it's name: otherwise it would throw `error: attribute 'name' missing`
getchoo commented 2024-05-07 06:24:08 +00:00 (Migrated from github.com)
Review
        nixpkgs.lib.genAttrs
        nixpkgs.lib.systems.flakeExposed
        (system: function nixpkgs.legacyPackages.${system});
```suggestion nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed (system: function nixpkgs.legacyPackages.${system}); ```
getchoo commented 2024-05-07 06:24:50 +00:00 (Migrated from github.com)
Review

im not really sure why this is set?

im not really sure why this is set?
getchoo commented 2024-05-07 06:25:15 +00:00 (Migrated from github.com)
Review
          inputsFrom = [ packages.${pkgs.system}.hello ];
```suggestion inputsFrom = [ packages.${pkgs.system}.hello ]; ```
getchoo commented 2024-05-07 06:26:34 +00:00 (Migrated from github.com)
Review
      overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; };

we really only need to use final when passing arguments to callPackage

```suggestion overlays.default = final: prev: { hello = prev.callPackage ./default.nix { }; }; ``` we really only need to use `final` when passing arguments to callPackage
getchoo commented 2024-05-07 06:27:21 +00:00 (Migrated from github.com)
Review

this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use nix run and nix run .#hello without this

```suggestion ``` this isn't what the app output is really meant for, and doesn't actually introduce anything new. users can still use `nix run` and `nix run .#hello` without this
Sigmanificient commented 2024-05-07 08:25:22 +00:00 (Migrated from github.com)
Review

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

To me it would make more sense for this to be dynamic, because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell
getchoo commented 2024-05-07 09:49:10 +00:00 (Migrated from github.com)
Review

because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell

slightly different builds of the same package. this is done in prismlauncher for example

this also duplicates inputs due to the default package (which only really slows eval time a bit, but still) and currently doesn't work at all

lib.attrValues packages evaluates to [ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]

> because i cant think of a scenario where you wouldn't add the build inputs of a package within the dev shell slightly different builds of the same package. this is done in [prismlauncher](https://github.com/prismlauncher/prismlauncher) for example this also duplicates inputs due to the `default` package (which only really slows eval time a bit, but still) and currently doesn't work at all `lib.attrValues packages` evaluates to `[ { default = { ... }; hello = { ... }; } { default = { ... }; hello = { ... }; } { ... } ]`