Skip to content
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.

Sign up for GitHub

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

Jump to bottom

nixos/xfce4-14: add xfce4-volumed-pulse when not using the desktop #70537

Merged
merged 1 commit into from Oct 6, 2019
Merged

nixos/xfce4-14: add xfce4-volumed-pulse when not using the desktop #70537

merged 1 commit into from Oct 6, 2019

Conversation

symphorien
Copy link
Member

xfce4-volumed-pulse is not abandoned, but is superseded by a panel
plugin which is not available when not using the desktop.
Source: https://xfce.org/about/news/?post=1565568000

Fixes the following issue: the volume up and volume down keys would not have any effect.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

worldofpeace
worldofpeace suggested changes Oct 6, 2019
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit message should lead-in with nixos/xfce4-14.

Can we add a comment that it's a "superfluous additional daemon" that only makes sense to add if we're not using the desktop? Conditional logic might not communicate this down the timeline.

Perhaps in a let in have it as pulseaudioDaemon = .... with the comment for clarity.

@worldofpeace
Copy link
Contributor

I'm in favor of backporting this. IIRC the old module added it.

@symphorien
nixos/xfce4-14: add xfce4-volumed-pulse when not using the desktop
d9cac95 
xfce4-volumed-pulse is not abandoned, but is superseded by a panel
plugin which is not available when not using the desktop.

Fixes: volume up/down keys support
@symphorien
Copy link
Member Author

Commit message should lead-in with nixos/xfce4-14.

Can we add a comment that it's a "superfluous additional daemon" that only makes sense to add if we're not using the desktop? Conditional logic might not communicate this down the timeline.

Perhaps in a let in have it as pulseaudioDaemon = .... with the comment for clarity.

I added a comment. Not let because the two alternative are not "homogeneous" in my opinion.

I am also in favor of backporting it. That was a regression for me when upgrading from 19.03.

@worldofpeace worldofpeace changed the title xfce4-14: add xfce4-volumed-pulse when not using the desktop nixos/xfce4-14: add xfce4-volumed-pulse when not using the desktop Oct 6, 2019
@worldofpeace
Copy link
Contributor

@GrahamcOfBorg test xfce4-14

@worldofpeace worldofpeace merged commit 8256a90 into NixOS:master Oct 6, 2019
@worldofpeace
Copy link
Contributor

backported in 8244f41

@symphorien symphorien deleted the xfce-media-keys branch March 21, 2020 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@worldofpeace worldofpeace worldofpeace requested changes

Assignees
No one assigned
Labels
6.topic: nixos 6.topic: xfce The Xfce Desktop Environment 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@symphorien @worldofpeace