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.

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

alsaequal: init at 0.6 #82003

Merged
merged 2 commits into from Mar 31, 2020
Merged

alsaequal: init at 0.6 #82003

merged 2 commits into from Mar 31, 2020

Conversation

ymeister
Copy link
Contributor

@ymeister ymeister commented Mar 7, 2020

Motivation for this change

Project description
Alsaequal is a real-time adjustable equalizer plugin for ALSA. It can be adjusted using an ALSA compatible mixer, like alsamixergui or alsamixer.

Alsaequal uses the Eq CAPS LADSPA Plugin as it's default equalizer but you can change it to use almost any LADSPA plugin, like mbeq from the swh-plugin package. Though alsaequal is primarily intended to be used as an equalizer you should be able to use it to control any LADSPA plugin. It's similar in functionality to the LADSPA plugin provided with ALSA but allows for real-time controls as opposed to static controls defined in the asoundrc file.

Metadata

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

doronbehar
doronbehar reviewed Mar 8, 2020
View reviewed changes
pkgs/tools/audio/alsaequal/default.nix Outdated Show resolved Hide resolved
pkgs/tools/audio/alsaequal/default.nix Outdated Show resolved Hide resolved
pkgs/tools/audio/alsaequal/default.nix Outdated Show resolved Hide resolved
pkgs/tools/audio/alsaequal/default.nix Show resolved Hide resolved
doronbehar
doronbehar requested changes Mar 8, 2020
View reviewed changes
pkgs/tools/audio/alsaequal/default.nix Outdated Show resolved Hide resolved
doronbehar
doronbehar requested changes Mar 9, 2020
View reviewed changes
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Besides that it's perfect .

maintainers/maintainer-list.nix Show resolved Hide resolved
pkgs/tools/audio/alsaequal/default.nix Show resolved Hide resolved
@doronbehar

This comment has been minimized.

Sign in to view
@doronbehar
Copy link
Contributor

Oh and please squash it all to 1 commit with just the message alsaequal: init at 0.6 once you are done.

Correction:

Please squash it all to 2 commits with just the messages:

maintainers: add ymeister
alsaequal: init at 0.6

Eventually.

@ymeister
Copy link
Contributor Author

ymeister commented Mar 9, 2020 •
edited

Oh and please squash it all to 1 commit with just the message alsaequal: init at 0.6 once you are done.

Correction:

Please squash it all to 2 commits with just the messages:

maintainers: add ymeister
alsaequal: init at 0.6

Eventually.

When I first thought of adding this package, I didn't expect to do so many changes to this pull request, so every commit I made was made via github, and now I am not sure if what you ask is possible without fully redoing the pull request.

@doronbehar
Copy link
Contributor

When I first thought of adding this package, I didn't expect to do so many changes to this pull request, so every commit I made was made via github, and now I am not sure if what you ask is possible without fully redoing the pull request.

If your local copy is not updated to what's here and you want to squash it all, use something along the lines of:

git pull ymeister master
# Do as explained here: https://stackoverflow.com/questions/5189560/squash-my-last-x-commits-together-using-git

BTW, tip: Use a different branch then master in your copy when creating PRs (not just in Nixpkgs) and use git push ymeister feature-branch.

doronbehar
doronbehar requested changes Mar 9, 2020
View reviewed changes
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Sorry that this makes your nice man page a work wasted because of the different format :/.

pkgs/tools/audio/alsaequal/alsaequal.1 Outdated Show resolved Hide resolved
@ymeister
Copy link
Contributor Author

All done. Sorry for taking so much time for such a trivial package.

doronbehar
doronbehar requested changes Mar 12, 2020
View reviewed changes
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Small nitpick and you are done.

pkgs/tools/audio/alsaequal/default.nix Outdated Show resolved Hide resolved
@doronbehar
Copy link
Contributor

All done. Sorry for taking so much time for such a trivial package.

Nothing is trivial in Nixos :). I thank you for your contribution and patience.

@doronbehar
Copy link
Contributor

Oh please squash that last commit @ymeister . so it'll be 2 commits in total, as requested earlier.

ymeister and others added 2 commits March 13, 2020 13:38
@ymeister @doronbehar
alsaequal: init at 0.6
5de1b16 
Co-Authored-By: Doron Behar <doron.behar@gmail.com>
@ymeister
maintainers: add ymeister
a0c9e95
@ymeister
Copy link
Contributor Author

@doronbehar , considering how much time have passed, I think you should re-approve changes to include squashed commits for this pull request to go through.

timokau
timokau approved these changes Mar 31, 2020
View reviewed changes
Copy link
Member

@timokau timokau left a comment

Choose a reason for hiding this comment

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

Thanks! Also to @doronbehar for the extensive review.

maintainers/maintainer-list.nix Show resolved Hide resolved
@timokau timokau merged commit da0fc06 into NixOS:master Mar 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@timokau timokau timokau approved these changes

@doronbehar doronbehar doronbehar approved these changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ymeister @doronbehar @timokau