Skip to content

pulse-visualizer: init at 1.2.2 #432723

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fndov
Copy link
Member

@fndov fndov commented Aug 11, 2025

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@fndov fndov marked this pull request as draft August 11, 2025 07:02
@nixpkgs-ci nixpkgs-ci bot added 12.first-time contribution This PR is the author's first one; please be gentle! 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` labels Aug 11, 2025
@fndov fndov force-pushed the init-pulse-visualizer branch from f02d95d to 1503c0a Compare August 11, 2025 08:46
@fndov
Copy link
Member Author

fndov commented Aug 11, 2025

@NixOS Can I have help with the formatting error?

Copy link
Contributor

@yzhou216 yzhou216 left a comment

Choose a reason for hiding this comment

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

Welcome!

Comment on lines 62 to 76
meta = with lib; {
description = "Real-time audio visualizer inspired by MiniMeters";
homepage = "https://github.com/Beacroxx/pulse-visualizer";
license = licenses.gpl3;
maintainers = [ maintainers.miyu ];
platforms = [
"x86_64-linux"
"aarch64-linux"
];
};

This comment was marked as resolved.

src = fetchFromGitHub {
owner = "Beacroxx";
repo = "pulse-visualizer";
rev = "882d6072f651ae00fcba66c9c2d5df2835b6cf61";

This comment was marked as resolved.


stdenv.mkDerivation (finalAttrs: {
pname = "pulse-visualizer";
version = "1.0.1";

This comment was marked as resolved.

Comment on lines 53 to 57
--replace-warn " -march=native" "" \
--replace-warn " -mtune=native" "" \
--replace-warn "-Wl,-s" "" \
--replace-warn " -s" "" \
--replace-warn 'set(CMAKE_INSTALL_PREFIX "/usr" CACHE PATH "Installation prefix" FORCE)' ""

This comment was marked as resolved.

@yzhou216
Copy link
Contributor

@NixOS Can I have help with the formatting error?

Did you try formatting with nixfmt?

@fndov
Copy link
Member Author

fndov commented Aug 11, 2025

@NixOS Can I have help with the formatting error?

Did you try formatting with nixfmt?

I believe I've formatted my files before every push, but it doesn't seem to make any changes

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a separate commit as maintainers: add miyu.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is why the CI is failing.

@nixpkgs-ci nixpkgs-ci bot removed the 12.first-time contribution This PR is the author's first one; please be gentle! label Aug 11, 2025
@fndov fndov force-pushed the init-pulse-visualizer branch from 8e4da74 to b3e30ab Compare August 11, 2025 23:52
@fndov
Copy link
Member Author

fndov commented Aug 11, 2025

oh no, it looks like I changed another file by accident, I'll see if I can fix that

@nixpkgs-ci nixpkgs-ci bot removed the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Aug 11, 2025
@fndov fndov force-pushed the init-pulse-visualizer branch from b3e30ab to 240c4f3 Compare August 11, 2025 23:59
@fndov
Copy link
Member Author

fndov commented Aug 12, 2025

Okay, I've been added to the maintainers list in a separate pull request, and I've updated the build with everything suggested, I've rebased to make it one commit, this is ready now.

@fndov fndov marked this pull request as ready for review August 12, 2025 00:09
@fndov fndov changed the title pulse-visualizer: init at 1.0.1 pulse-visualizer: init at 1.1.0 Aug 12, 2025
@yzhou216

This comment was marked as resolved.

@fndov
Copy link
Member Author

fndov commented Aug 12, 2025

That's unfortunate, can you guide me to make the package x86_64 exclusive?

Copy link
Contributor

@yzhou216 yzhou216 left a comment

Choose a reason for hiding this comment

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

That's unfortunate, can you guide me to make the package x86_64 exclusive?

Sure

@yzhou216
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 432723

Logs: https://github.com/yzhou216/nixpkgs-review-gha/actions/runs/16896773056


x86_64-linux

✅ 1 package built:
  • pulse-visualizer

aarch64-linux

No rebuilds


x86_64-darwin

No rebuilds


aarch64-darwin

No rebuilds

Copy link
Contributor

@yzhou216 yzhou216 left a comment

Choose a reason for hiding this comment

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

Approved automatically following the successful run of nixpkgs-review.

@fndov fndov force-pushed the init-pulse-visualizer branch from 815f515 to f8ef14c Compare August 12, 2025 02:19
@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 12, 2025
@Vuks69
Copy link
Contributor

Vuks69 commented Aug 18, 2025

Just a reminder that @Audio-Solutions organization is now the owner.

@fndov fndov force-pushed the init-pulse-visualizer branch from f8ef14c to 4778bc0 Compare August 19, 2025 04:44
@fndov fndov changed the title pulse-visualizer: init at 1.1.0 pulse-visualizer: init at 1.2.2 Aug 19, 2025
@fndov
Copy link
Member Author

fndov commented Aug 19, 2025

@NixOS Can we merge this please? It's been a week, let me know if there's anything I can do.

Append to maintainers list

Append to maintainers list V2

Append new line to package

Rebase

Apply suggested changes

Update platforms

Co-authored-by: Yiyu Zhou <yiyuzhou19@gmail.com>

Update platforms V2

Co-authored-by: Yiyu Zhou <yiyuzhou19@gmail.com>

pulse-visualizer: init at 1.2.2

Fix minor issue with version number, extra v
@fndov fndov force-pushed the init-pulse-visualizer branch from 4778bc0 to b7b779c Compare August 19, 2025 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. 12.approvals: 1 This PR was reviewed and approved by one person.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants