Skip to content

wolf-spectrum: init at 1.0.0#83579

Closed
matthiasbeyer wants to merge 2 commits intoNixOS:masterfrom
matthiasbeyer:add-wolf-spectrum
Closed

wolf-spectrum: init at 1.0.0#83579
matthiasbeyer wants to merge 2 commits intoNixOS:masterfrom
matthiasbeyer:add-wolf-spectrum

Conversation

@matthiasbeyer
Copy link
Copy Markdown
Contributor

Thanks for @mrVanDalo as this is only copy-paste from here: https://git.ingolf-wagner.de/palo/nixos-config/src/master/pkgs/wolf-spectrum/default.nix

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.

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Mar 28, 2020
@ofborg ofborg bot requested a review from magnetophon March 28, 2020 09:32
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 28, 2020
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You might wanna check the homepage, description and maintainer.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Whops, sorry 'bout that.

@ofborg ofborg bot added the 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. label Mar 28, 2020
@magnetophon
Copy link
Copy Markdown
Member

Works for me!

Comment on lines 38 to 46
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is there anyway to check that these are the correct install paths? Also, i don't see any headers being installed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Why would you need any headers?

What do you mean by correct install paths? I can tell you that this plugin worked in ardour5 with this setup... not sure what you want. 😄

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess that's what i was looking for

@magnetophon magnetophon mentioned this pull request Jun 5, 2020
10 tasks
@stale
Copy link
Copy Markdown

stale bot commented Sep 27, 2020

Hello, I'm a bot and I thank you in the name of the community for your contributions.

Nixpkgs is a busy repository, and unfortunately sometimes PRs get left behind for too long. Nevertheless, we'd like to help committers reach the PRs that are still important. This PR has had no activity for 180 days, and so I marked it as stale, but you can rest assured it will never be closed by a non-human.

If this is still important to you and you'd like to remove the stale label, we ask that you leave a comment. Your comment can be as simple as "still important to me". But there's a bit more you can do:

If you received an approval by an unprivileged maintainer and you are just waiting for a merge, you can @ mention someone with merge permissions and ask them to help. You might be able to find someone relevant by using Git blame on the relevant files, or via GitHub's web interface. You can see if someone's a member of the nixpkgs-committers team, by hovering with the mouse over their username on the web interface, or by searching them directly on the list.

If your PR wasn't reviewed at all, it might help to find someone who's perhaps a user of the package or module you are changing, or alternatively, ask once more for a review by the maintainer of the package/module this is about. If you don't know any, you can use Git blame on the relevant files, or GitHub's web interface to find someone who touched the relevant files in the past.

If your PR has had reviews and nevertheless got stale, make sure you've responded to all of the reviewer's requests / questions. Usually when PR authors show responsibility and dedication, reviewers (privileged or not) show dedication as well. If you've pushed a change, it's possible the reviewer wasn't notified about your push via email, so you can always officially request them for a review, or just @ mention them and say you've addressed their comments.

Lastly, you can always ask for help at our Discourse Forum, or more specifically, at this thread or at #nixos' IRC channel.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 27, 2020
@magnetophon
Copy link
Copy Markdown
Member

still important to me

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 28, 2020
@ryantm ryantm added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 3, 2020
@mrVanDalo
Copy link
Copy Markdown
Contributor

@matthiasbeyer there is a merge conflict here.

@ofborg ofborg bot added 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Nov 25, 2020
@thiagokokada
Copy link
Copy Markdown
Contributor

thiagokokada commented Dec 31, 2020

Result of nixpkgs-review pr 83579 1

1 package built:
  • wolf-spectrum

Don't know how to properly test this, but I ran the binary and it runs (with error because I probably doesn't have a proper JACK setup).

@nixos-discourse
Copy link
Copy Markdown

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/313

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Randomly commented-out stuff, and this installPhase in general: Please add some comments that describe what's (not) copied and why.

I suspect from "BUILD_DSSI=false" above that it doesn't build and you might want it to build in the future -- a comment on "BUILD_DSSI=false" might be excellent for that (you may also want to make the makeFlags above multi-line to best be able to place your comment).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
#cp -r bin/wolf-spectrum-dssi* $out/lib/dssi/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You forgot to remove the comment.

Comment on lines 39 to 42
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
mkdir -p $out/lib/lv2
#mkdir -p $out/lib/dssi
mkdir -p $out/lib/vst
mkdir -p $out/bin/
mkdir -p $out/lib/lv2 $out/lib/vst $out/bin/

Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
description = "Real-time 2D spectrogram plugin (LV2, VST and Jack)";
license = licenses.gpl3Plus;
maintainers = [ maintainers.matthiasbeyer ];
platforms = [ "i686-linux" "x86_64-linux" ];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
platforms = [ "i686-linux" "x86_64-linux" ];
platforms = platforms.linux;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You forgot to remove the comment.

@matthiasbeyer matthiasbeyer deleted the add-wolf-spectrum branch April 13, 2021 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants