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

gruvbox-material-icons-gtk: init at unstable-2021-07-25 #188402

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xerbalind
Copy link

Description of changes

Gruvbox material themed icons from https://github.com/TheGreatMcPain/gruvbox-material-gtk.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

Copy link
Member

@azahi azahi left a comment

Choose a reason for hiding this comment

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

Also, please make sure you only have two commits.

pkgs/data/icons/gruvbox-material-icons-gtk/default.nix Outdated Show resolved Hide resolved
pkgs/data/icons/gruvbox-material-icons-gtk/default.nix Outdated Show resolved Hide resolved
pkgs/data/icons/gruvbox-material-icons-gtk/default.nix Outdated Show resolved Hide resolved
sha256="sha256-1O34p9iZelqRFBloOSZkxV0Z/7Jffciptpj3fwXPHbc=";
};

nativeBuildInputs = [ gtk3 ];
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing you meant this.

Suggested change
nativeBuildInputs = [ gtk3 ];
buildInputs = [ gtk3 ];

Copy link
Author

Choose a reason for hiding this comment

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

This is better as gtk3 is only being used at build-time: gtk-update-icon-cache

Copy link
Member

Choose a reason for hiding this comment

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

I see. Alright.

pkgs/data/icons/gruvbox-material-icons-gtk/default.nix Outdated Show resolved Hide resolved
pkgs/data/icons/gruvbox-material-icons-gtk/default.nix Outdated Show resolved Hide resolved
description = "Gruvbox material icons for GTK based desktop environments";
homepage = "https://github.com/TheGreatMcPain/gruvbox-material-gtk";
license = licenses.mit;
platforms = platforms.unix;
Copy link
Member

Choose a reason for hiding this comment

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

This is the default.

Suggested change
platforms = platforms.unix;
platforms = platforms.unix;

Copy link
Author

Choose a reason for hiding this comment

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

Ah didn't know thanks.

Copy link
Member

Choose a reason for hiding this comment

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

My intention was to suggest you to remove this line because it's not needed. :^)

@@ -25283,6 +25283,10 @@ with pkgs;
inherit (plasma5Packages) breeze-icons;
};

gruvbox-material-icons-gtk = callPackage ../data/icons/gruvbox-material-icons-gtk {
inherit (plasma5Packages) breeze-icons;
Copy link
Member

Choose a reason for hiding this comment

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

I think plasma5Packages shouldn't be used here.

Copy link
Author

Choose a reason for hiding this comment

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

I assumed, as breeze-icons sits in the scope of libsForQt5 and I noticed others are using plasma5Packages for breeze-icons too.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. I was just thinking that it being an icon set, it should be DE-agnostic.

@xerbalind xerbalind changed the title gruvbox-material-icons-gtk: init at latest-2021-07-25 gruvbox-material-icons-gtk: init at unstable-2021-07-25 Aug 27, 2022
@xerbalind xerbalind requested a review from azahi August 27, 2022 21:11
Copy link
Member

@azahi azahi left a comment

Choose a reason for hiding this comment

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

Don't forget to squash your commits.

description = "Gruvbox material icons for GTK based desktop environments";
homepage = "https://github.com/TheGreatMcPain/gruvbox-material-gtk";
license = licenses.mit;
platforms = platforms.unix;
Copy link
Member

Choose a reason for hiding this comment

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

My intention was to suggest you to remove this line because it's not needed. :^)

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin and removed 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 labels Aug 27, 2022
@xerbalind xerbalind force-pushed the master branch 2 times, most recently from 754b1fa to 2f28b1f Compare August 28, 2022 19:45
@ofborg ofborg bot added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 10.rebuild-linux: 1-10 10.rebuild-linux: 1 and removed 10.rebuild-linux: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Aug 28, 2022
@xerbalind xerbalind requested a review from azahi August 28, 2022 20:18
Comment on lines 16126 to 16127
eclint = callPackage ../development/tools/eclint { };

Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this is merge artifact?

Copy link
Author

Choose a reason for hiding this comment

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

ah yes I see it now

Copy link
Author

Choose a reason for hiding this comment

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

should be fixed now

@azahi
Copy link
Member

azahi commented Aug 28, 2022

Result of nixpkgs-review pr 188402 run on x86_64-linux 1

1 package built:
  • gruvbox-material-icons-gtk

@azahi azahi added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Aug 28, 2022
@leona-ya leona-ya mentioned this pull request Jan 31, 2024
13 tasks
@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
@ghost ghost closed this Dec 19, 2024
@ghost ghost reopened this Jan 22, 2025
@github-actions github-actions bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Jan 22, 2025
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 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 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants