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

waybar: libcava 0.10.2 -> 0.10.3 #345774

Merged
merged 1 commit into from
Oct 5, 2024
Merged

waybar: libcava 0.10.2 -> 0.10.3 #345774

merged 1 commit into from
Oct 5, 2024
+2 −2

Conversation

i-am-logger
Copy link
Contributor

@i-am-logger i-am-logger commented Oct 1, 2024 •
edited
Loading

Update libcava to version 0.10.3

Updated the libcava dependency in waybar to version 0.10.3 to include the latest fixes and improvements.
Via Alexays/Waybar#3426 (comment)

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@i-am-logger i-am-logger force-pushed the waybar-update-libcava-0.10.3 branch from 472f8b1 to aa5441f Compare October 1, 2024 20:52
@i-am-logger
Copy link
Contributor Author

i-am-logger commented Oct 1, 2024 •
edited
Loading

@NixOS/nix-formatting - seems there is an issue with the formatting of nixfmt...

using nixfmt v0.6.0

@infinisil
Copy link
Member

Please go to the Nixpkgs root directory, run nix-shell, then:

nixfmt 'pkgs/by-name/wa/waybar/package.nix'

Did you do that? :)

khaneliman
khaneliman requested changes Oct 1, 2024
View reviewed changes
Copy link
Contributor

@khaneliman khaneliman left a comment

Choose a reason for hiding this comment

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

please format with nixfmt-rfc-style and formatting changes should be separate from the actual change

@i-am-logger
Copy link
Contributor Author

i-am-logger commented Oct 1, 2024 •
edited
Loading

Please go to the Nixpkgs root directory, run nix-shell, then:

nixfmt 'pkgs/by-name/wa/waybar/package.nix'

Did you do that? :)

yes

image

@i-am-logger i-am-logger force-pushed the waybar-update-libcava-0.10.3 branch from aa5441f to d82fde0 Compare October 1, 2024 21:15
@i-am-logger
Copy link
Contributor Author

thank you @khaneliman that did the trick

@infinisil
Copy link
Member

run nix-shell

This is the important part! There's a shell.nix in the Nixpkgs root directory that gives you the correct nixfmt version. Notably even pkgs.nixfmt-rfc-style might not be the right version.

@khaneliman
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 345774

x86_64-linux

✅ 1 package built:
  • waybar

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Oct 2, 2024
@khaneliman
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 345774

x86_64-linux

✅ 1 package built:
  • waybar

@Frontear
Copy link
Member

Frontear commented Oct 2, 2024

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 345774

x86_64-linux

✅ 1 package built:

woah is that a new format for that command? looks awesome

@khaneliman
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.
Command: nixpkgs-review pr 345774

x86_64-linux

✅ 1 package built:

woah is that a new format for that command? looks awesome

Mic92/nixpkgs-review#419 using gaetans PR that supports multi platform reviews and has some nice new formatting

@i-am-logger
Copy link
Contributor Author

Result of nixpkgs-review pr 345774 run on aarch64-linux 1

1 package built:
  • waybar

@i-am-logger
Copy link
Contributor Author

Result of nixpkgs-review pr 345774 run on aarch64-darwin 1

@i-am-logger
Copy link
Contributor Author

Result of nixpkgs-review pr 345774 run on x86_64-darwin 1

SuperSandro2000
SuperSandro2000 requested changes Oct 2, 2024
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Please follow the contributing guide when naming your commits.

@wegank wegank added 12.approvals: 1 This PR was reviewed and approved by one reputable person 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Oct 2, 2024
@i-am-logger i-am-logger force-pushed the waybar-update-libcava-0.10.3 branch from d82fde0 to 1604bca Compare October 2, 2024 14:54
@i-am-logger i-am-logger force-pushed the waybar-update-libcava-0.10.3 branch from 1604bca to 12d2aa5 Compare October 2, 2024 14:55
@i-am-logger i-am-logger force-pushed the waybar-update-libcava-0.10.3 branch from 12d2aa5 to 7f9cb24 Compare October 2, 2024 15:09
@i-am-logger
Copy link
Contributor Author

Please follow the contributing guide when naming your commits.

fixed, please let me know if i missed something

@Frontear
Copy link
Member

Frontear commented Oct 2, 2024

Rename your commit to waybar: 0.10.2 -> 0.10.3. This follows more closely to contributing guidelines

Frontear
Frontear reviewed Oct 2, 2024
View reviewed changes
Copy link
Member

@Frontear Frontear left a comment

Choose a reason for hiding this comment

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

Have you reviewed if the patches are still necessary? If they aren't could you have them removed?

@ofborg ofborg bot requested a review from khaneliman October 2, 2024 17:18
@i-am-logger
waybar: libcava 0.10.2 -> 0.10.3
f00a2b7 
Updated the libcava dependency in waybar to version 0.10.3 to include the latest fixes and improvements.
Via Alexays/Waybar#3426 (comment)
@i-am-logger i-am-logger force-pushed the waybar-update-libcava-0.10.3 branch from 7f9cb24 to f00a2b7 Compare October 2, 2024 17:38
@i-am-logger i-am-logger changed the title waybar: Update libcava to version 0.10.3 waybar: libcava 0.10.2 -> 0.10.3 Oct 2, 2024
@i-am-logger
Copy link
Contributor Author

Rename your commit to waybar: 0.10.2 -> 0.10.3. This follows more closely to contributing guidelines

thanks, added libcava as the commit update only this dependency

@i-am-logger
Copy link
Contributor Author

Have you reviewed if the patches are still necessary? If they aren't could you have them removed?

no, i have not as the fix update a single dependency and nothing else

@khaneliman
Copy link
Contributor

Have you reviewed if the patches are still necessary? If they aren't could you have them removed?

The patch is necessary until the package version is bumped..

khaneliman
khaneliman approved these changes Oct 2, 2024
View reviewed changes
Copy link
Contributor

@khaneliman khaneliman left a comment

Choose a reason for hiding this comment

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

Still looks fine to me, we haven't cut a new release upstream, yet. But, this can be updated independently.

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 345774

x86_64-linux

✅ 1 package built:
  • waybar

aarch64-linux

✅ 1 package built:
  • waybar

x86_64-darwin

aarch64-darwin

@ofborg ofborg bot requested a review from khaneliman October 2, 2024 20:22
@i-am-logger
Copy link
Contributor Author

Please follow the contributing guide when naming your commits.

@SuperSandro2000 can you review again?

Frontear
Frontear approved these changes Oct 4, 2024
View reviewed changes
Copy link
Member

@Frontear Frontear left a comment

Choose a reason for hiding this comment

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

approved! also apologies about the patch comment i completely misread the derivation. i see that the patches are for waybar, not libcava 😅

@wegank wegank added 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people and removed 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Oct 4, 2024
@SuperSandro2000 SuperSandro2000 merged commit 1da7ac6 into NixOS:master Oct 5, 2024
30 checks passed
@i-am-logger i-am-logger deleted the waybar-update-libcava-0.10.3 branch October 6, 2024 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GaetanLepage GaetanLepage GaetanLepage approved these changes

@Frontear Frontear Frontear approved these changes

@SuperSandro2000 SuperSandro2000 SuperSandro2000 approved these changes

@lovesegfault lovesegfault Awaiting requested review from lovesegfault

@FlorianFranzen FlorianFranzen Awaiting requested review from FlorianFranzen

@rodrgz rodrgz Awaiting requested review from rodrgz

@Synthetica9 Synthetica9 Awaiting requested review from Synthetica9

@minijackson minijackson Awaiting requested review from minijackson

@khaneliman khaneliman Awaiting requested review from khaneliman

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 12.approvals: 3+ This PR was reviewed and approved by three or more reputable people 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@i-am-logger @infinisil @khaneliman @Frontear @GaetanLepage @SuperSandro2000 @wegank