-
-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
pommed service: Replace broken pommed package with pommed-light #21267
Conversation
7bc58a3
to
96d4793
Compare
substituteInPlace pommed.conf.mactel --replace /usr $out | ||
substituteInPlace pommed.conf.pmac --replace /usr $out | ||
substituteInPlace pommed/beep.h --replace /usr $out | ||
substituteInPlace pommed/evdev.h --replace 0262 0252 |
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.
line 28: I added this substitution due to a personal bug that I had found (see bytbox/pommed-light#31). Once a new version with a fix is published, I will remove this line. The bug probably affects a small group of people though (those with Macbook Pro 8.2)..
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've removed this line on second thought, since I'm not sure whether it wouldn't break things for others.
The pommed package was marked as broken. It is also severely unmaintained. I therefore chose to replace it entirely with `pommed-light`, for now.
96d4793
to
8287dc2
Compare
install -Dm644 pommed/data/click.wav $out/share/pommed/click.wav | ||
''; | ||
|
||
meta = { |
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.
Perhaps add meta.platforms = stdenv.lib.platforms.linux
.
Also, have you considered adding yourself as the maintainer?
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 have, but I don't feel like taking such a responsibility at the moment.
I will add your suggestion.
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.
Fair enough. Being listed as a maintainer primarily means that you're somebody who cares about the package and might be able to answer questions about it; beyond that, it means whatever you want it to.
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.
Yea I understand. I don't think it's something I will commit to long term (e.g. at least half a year or so), so that's why I prefer not to list myself as maintainer. In any case the @mention-bot would just tag me by analyzing the history when there's an issue right?
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've never seen it do that for ordinary issues, but it usually works for PRs.
Looks pretty good to me, though I can't speak to the specifics. If nobody objects, I'll integrate this soon. |
d20345e
to
8ffa6c5
Compare
Merged. Thank you. |
Motivation for this change
The pommed package was marked as broken. It is also severely unmaintained. I therefore chose to replace it entirely with
pommed-light
, for now.There are some forks of the original pommed to be found (in fact,
pommed-light
seems to be one) which aren't unmaintained. For example, Arch's AUR has a packagepommed-jalaziz
which seems to be maintained well.So we could maybe follow the same example, and add two versions of pommed in our package tree:
pommed-jalaziz
andpommed-light
. And then we could extend the service with an option (e.g.services.hardware.pommed.package
) to allow the user to select a package.Things done
(nix.useSandbox on NixOS,
or option
build-use-sandbox
innix.conf
on non-NixOS)
nix-shell -p nox --run "nox-review wip"
./result/bin/
)