-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
awakened-poe-trade: init at 3.25.102 #341252
base: master
Are you sure you want to change the base?
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.
meta.changelog
and meta.sourceProvenance
missing
d0ee864
to
482e98e
Compare
Renamed the package to go under |
Commit message and PR title are wrong. |
482e98e
to
e98e53f
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.
description = "Path of Exile trading app for price checking"; | ||
homepage = "https://github.com/SnosMe/awakened-poe-trade"; | ||
changelog = "https://github.com/SnosMe/awakened-poe-trade/releases/tag/v${version}"; | ||
sourceProvenance = [ lib.sourceTypes.binaryBytecode ]; |
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.
What about it is bytecode?
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.
I wasn't sure which option to use. Also, thank you for pointing out that I use pname way too much, I'll change that as well. Should anything else be changed in the code I committed, or would it be good after both these changes?
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.
Side-note: should I ignore this warning?
❯ ./result/bin/awakened-poe-trade
hook_thread_proc [101]: Could not set thread priority 49 for thread 0x7F0BBEC7F6C0!
load_input_helper [1827]: XkbGetKeyboard failed to locate a valid keyboard!
APPIMAGE env is not defined, current application is not an AppImage
e98e53f
to
4c8f86f
Compare
name = "${pname}-${version}"; | ||
inherit src; |
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 fails evaluation for me. I think, didn't test, you need to do today:
name = "${pname}-${version}"; | |
inherit src; | |
inherit pname src version; |
Description of changes
I've added Awakened POE (a Path of Exile trading app for price checking).
I ended up packaging it for Nix using the AppImage, as the build process for the app involves using a build script from upstream that uses
import electron
, which would use the Node.JS electron version and not take advantage of NixOS' dependency system. Upstream changes would likely need to be made to be packaged from source, or there is probably something I don't know (this is one of my first PRs to nixpkgs).Closes #302672.
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.