codd: init at 0.1.4 #338660
codd: init at 0.1.4 #338660
Conversation
github-actions
bot
added
the
8.has: maintainer-list (update)
ofborg
bot
added
8.has: package (new)
mzabani
force-pushed
the
add-codd-postgres-migration-tool
branch
from
d4e226a to
d841b63
Compare
|
@eclairevoyant what is the typical flow when adding new packages? Should I ping the Haskell maintainers? Or maybe I should ask in the discourse thread for a review? |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/4622 |
FliegendeWurst
added
the
11.by: upstream-developer
| "/SystemResourcesSpecs/" | ||
| ]; | ||
|
|
||
| description = "CLI tool that applies plain postgres SQL migrations atomically with strong and automatic cross-environment schema equality checks"; |
There was a problem hiding this comment.
This and below need to be wrapped in a meta = block.
There was a problem hiding this comment.
Thanks! I tried that but then building led to an error. Maybe Haskell's mkDerivation is different and requires these as top-level attributes? That's what it seems like from looking at other Haskell packages like Koka:
https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/koka/default.nix#L100-L107
I rebased to resolve the conflict and pushed again just in case.
There was a problem hiding this comment.
Oh, I didn't see that. Then it is okay.
wegank
added
the
2.status: merge conflict
mzabani
force-pushed
the
add-codd-postgres-migration-tool
branch
from
d841b63 to
f7af0ba
Compare
pkgs/top-level/all-packages.nix
Outdated
| @@ -6130,6 +6130,8 @@ with pkgs; | |||
| clangStdenv = if stdenv.cc.isClang then stdenv else lowPrio llvmPackages.stdenv; | |||
| libcxxStdenv = if stdenv.hostPlatform.isDarwin then stdenv else lowPrio llvmPackages.libcxxStdenv; | |||
|
|
|||
| codd = haskell.lib.compose.justStaticExecutables (haskellPackages.callPackage ../development/tools/database/codd { }); | |||
There was a problem hiding this comment.
Should be in pkgs/by-name
There was a problem hiding this comment.
Thanks. I followed the pkgs/by-name README and some other packages as examples. I hope I did the right thing.
| "/SystemResourcesSpecs/" | ||
| ]; | ||
|
|
||
| description = "CLI tool that applies plain postgres SQL migrations atomically with strong and automatic cross-environment schema equality checks"; |
There was a problem hiding this comment.
Oh, I didn't see that. Then it is okay.
mzabani
force-pushed
the
add-codd-postgres-migration-tool
branch
from
8c9ab25 to
b81969f
Compare
pkgs/by-name/co/codd/generated.nix
Outdated
| typed-process | ||
| unliftio | ||
| ]; | ||
|
|
There was a problem hiding this comment.
| testToolDepends = [ haskellPackages.hspec-discover ]; |
There was a problem hiding this comment.
Thanks! I'm curious what this changes, as it seemed tests would run even if I didn't declare hspec-discover as a dependency.
There was a problem hiding this comment.
It only matters if you set strictDeps = true. I'm currently trying strictDepsByDefault = true, which has already found a lot of issues like this.
Co-authored-by: Arne Keller <[email protected]>
Co-authored-by: Arne Keller <[email protected]>
ofborg
bot
removed
the
2.status: merge conflict
| # This was generated with cabal2nix but contains important modifications like jailbreaking haxl | ||
| # and disabling tests selectively, among a few other minor things. | ||
| # Don't just blindly replace it with something generated by cabal2nix. |
There was a problem hiding this comment.
Have a look at https://github.com/NixOS/nixpkgs/tree/master/pkgs/by-name/ni/nixfmt-rfc-style to see how this can be split up in a purely generated file and overrides for the modifications. This even includes an update script, which will give us auto-updates.
Can we do something like that here, too?
There was a problem hiding this comment.
I took a look but having an additional script and running instructions felt a bit overkill. Unless this is the standard for all packages now, I'd prefer to keep this as is, particularly since I'm the author of the tool and will be keeping this up-to-date myself.
One intermediate approach is to do some splitting without a bash script, though. Let me know what you prefer!
There was a problem hiding this comment.
The update script is entirely optional. My main point is maintainability: It's currently impossible for somebody else except you to handle updates etc., because of "Don't just blindly replace it with something generated by cabal2nix.". Only you know which changes you intended and which would be because of an update.
So you should really split the generated code from the intentional overrides.
wegank
added
the
2.status: merge conflict
Description of changes
Codd is developed by me and is essentially a tool that applies postgresql SQL migrations. Its homepage is https://github.com/mzabani/codd
Feel free to be direct and point out anything wrong here. I never had contributions merged into nixpkgs before.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.