-
-
Notifications
You must be signed in to change notification settings - Fork 17.1k
Adding Atuin-Desktop to Nix Packages #448104
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
Conversation
|
This is my first attempt so please be gentle! I was able to build this locally on NixOS 25.05. |
|
Could you please follow the PR format. The PR title, and the commit names. Your commits should look like:
And only those two commits in that order. I also wonder why this is not built from source? Looks like it's possible. |
|
Sure. I'll go ahead and rebuild it. Honestly, I was a bit confused on what format to use with all the links in the contribution document. My apologies. Again, this is my first attempt at a nix package. |
|
It can be built from source. I am wrangling with some build issues now, though. It is possible to first only use the pre-built binary, and only then change to compiling from source when I or someone figures it out. |
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.
Hey, thank you for trying to help out. Please, first read the contributing guidelines in the repo to modify the PR accordingly. Some quick notes.
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.
Typo in the package name.
| version = "0.1.0"; | ||
|
|
||
| src = fetchurl { | ||
| url = "https://github.com/atuinsh/desktop/releases/download/v0.1.0/Atuin_Desktop_0.1.0_amd64.deb"; |
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.
You should reference ${version} from above to minimize repeating yourself.
| pkg-config, | ||
| }: | ||
|
|
||
| stdenv.mkDerivation rec { |
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.
Using (final: prev: { would be the preferred approach.
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.
- only overrides support two-arg lambdas, a primary mkDerivation call only supports
(final: {})-type lambdas - until those lambdas are made to propagate differently than rec (they currently don't) and are documented (they currently aren't), let's not suggest that: CONTRIBUTING: discourage style reviews not based on written rules #445898
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.
- True, we are using the default
mkDerivationhere. Never mind. - Sure.
Consider this issue resolved.
18cab22 to
1c26ea9
Compare
The following pull request is for adding Atuin-Desktop to nixpkgs. Atuin-Desktop is an Open Source app that provides executable runbooks that incorporate executable code into documentation. You can read more by clicking the links below:
Github Repo
Website Link
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.