Noise Explorer command-line tool #1
Conversation
noiseExplorer/default.nix
Outdated
| echo -n "[NoiseExplorer] Generating NoiseParser..." | ||
| echo "Parser Generated" | ||
| cd util | ||
| node --version |
There was a problem hiding this comment.
yes. Removed
|
|
||
| # Nixpkgs / NixOS version to use. | ||
| inputs.nixpkgs.url = "nixpkgs/nixos-20.09"; | ||
| inputs = { |
There was a problem hiding this comment.
don't you also need the source of the noise explorer as well ?
Perhaps you have it locally, but I don't think it's available to everybody.
Perhaps that explains why you needed to cd src further down.
There was a problem hiding this comment.
It's fetched via fetchgit in default.nix which seems fine.
But I agree, handling the source as a flake input will allow the user to change the source version via flakes interface (nix flake lock --update-input will be possible for example). That can be convenient, especially when this flake is used by other flakes.
There was a problem hiding this comment.
This is fetched in default.nix and try to use the same version in the flake.nix and also in default.nix
There was a problem hiding this comment.
Would you mind putting the src in the inputs ?
It's a little clearer that way, it also provides a convenient update mechanism.
There was a problem hiding this comment.
In the default.nix there seem to be many print statements. Probably leftovers form debugging. Maybe you can have another look and remove unnecessary things.
Usually phases announce themselves already. There is probably no need to have things like echo "Build Phase Over...."
Exposing the source as flake input can make usage more convenient as mentioned in the other comment.
Apart from that it looks good to me.
|
|
||
| # Nixpkgs / NixOS version to use. | ||
| inputs.nixpkgs.url = "nixpkgs/nixos-20.09"; | ||
| inputs = { |
There was a problem hiding this comment.
It's fetched via fetchgit in default.nix which seems fine.
But I agree, handling the source as a flake input will allow the user to change the source version via flakes interface (nix flake lock --update-input will be possible for example). That can be convenient, especially when this flake is used by other flakes.
|
I have made two more commits implementing the changes. |
|
Is it a good idea to reformat auto generated code with nixpkgs-fmt? Each time you re-run node2nix it will destroy the format again. One more thing I noticed. There is no meta attribute set for the derivation in default.nix. |
|
Right on topic for NixOS/rfcs#101 |
That only makes sense if the entire project agrees and is setup in such a way that unformatted Nix is rejected by CI, formatting is enforced with editor/pre-commit configuration etc. If you can't enforce it then it will diverge over time over and over again. |
|
We should definitely try adding a github action to enforce this check, as an experiment. |
|
@cdepillabout Planning to finalise this. Should i add "ready for review flag" |
|
@raghuramlavan When you're happy with this, please merge it in and send a message on ngi-nix/ngi#40. |
The flake for Noise Explorer command-line tool. It needs noise pattern files from Noise Explorer Repository. The tool needs rust and go for building and has been included in the development environment.