-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
postprocessd: init at 0.3.0 #338795
postprocessd: init at 0.3.0 #338795
Conversation
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.
Seems reasonable just wondering if the comment should be simplified and/or in the commit message instead.
pkgs/by-name/me/megapixels/search-for-postprocessors-in-NixOS-specific-global-location.patch
Outdated
Show resolved
Hide resolved
8cf4ab9
to
90b89f0
Compare
Yeah, the comment is pretty long. But I felt using these global paths goes against the NixOS philosophy and is therefore controversial, so I wanted to explain everything with as much detail as possible. Feel free to tell me any concrete suggestions you may have on how to simplify it. At first I had this text in the commit message but then I moved it into this comment because I didn't like how the code contained a patch without any explanation and users would have to search the Git history just to find an explanation why this patch exists and what it does. |
90b89f0
to
5ed3461
Compare
pkgs/by-name/me/megapixels/search-for-postprocessors-in-NixOS-specific-global-location.patch
Outdated
Show resolved
Hide resolved
5ed3461
to
2589012
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/1955 |
2589012
to
5be537b
Compare
I fixed a grammatical error in the comment. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/1974 |
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.
Let's rebase this on master to see if it still passes CI, and if so I believe we should merge this.
5be537b
to
07c8520
Compare
Rebased. |
Everything except the Darwin tests ran and this program is only meant for Linux. |
Thanks! |
Description of changes
Add postprocessd, a queueing Megapixels post-processor.
Also patch Megapixels to be able to use the new postprocessor if it is installed globally. See the long comment in the code for a more detailed description and a discussion of tradeoffs.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-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.