-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add initial packages to refactor out generic code from inferenceql.query
#1
Conversation
ee1a306
to
7b46e3a
Compare
cd0ef10
to
8fbe5cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. I left some inline comments.
nix build .#ociImgBase
and
nix build .#ociImgIqlQuer
and
nix develop
all work locally.
I wonder if we should rename this to inferenceql.nix. Cloning it locally into a directory called just nix feels kind of awkward.
# system. | ||
|
||
devShells.default = pkgs.mkShell { | ||
buildInputs = [] ++ toolkit; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why are doing this instead of [toolkit]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This expression is list concatenation, since toolkit
is already a list. Since we are concatenating an empty list this is in fact a no-op. This expression used to include some additional tools that are not part of the "standard" toolkit, but i left it in there to show where you could inject more deps if needed. I'll leave a comment to this effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the equivalent expression is buildInputs = toolkit;
and not buildInputs = [toolkit];
.
}; | ||
}; | ||
flake = { | ||
# The usual flake attributes can be defined here, including system- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mind linking some kind of reference here for all those Nix newbies like me out there? I actually don't know what this does...
CI just builds all artifacts.
8fbe5cb
to
fbc56a6
Compare
This repository will own InferenceQL organization-wide / cross-repository nix code, including utilities, infrastructure, and derivations for software artifacts that we don't directly own (such as Python packages that aren't already released in
nixpkgs
.The best-practice pattern for this repository is still being worked out, and will probably involve creating overlays for
nixpkgs
, so in the meantime this first PR just deprecates some of the reusable code ininferenceql/inferenceql.query
which can after this just call here.It is unclear if the leaf Docker image wants to live in this repository or that one; so for now, I have it in both places and we can see which one is more useable.