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

catppuccin-sddm: init at 1.0.0 #296682

Merged
merged 2 commits into from
May 12, 2024
Merged

catppuccin-sddm: init at 1.0.0 #296682

merged 2 commits into from
May 12, 2024

Conversation

ElysaSrc
Copy link
Contributor

@ElysaSrc ElysaSrc commented Mar 17, 2024

Description of changes

Adding package catppuccin-sddm on nixpkgs, since we already have some other catppuccin official themes (gtk, plasma, icons etc...).

I've added options to the derivation to control the theme file (flavor, theme.conf options...).

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.05 Release Notes (or backporting 23.05 and 23.11 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.

@keenanweaver
Copy link
Member

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

1 package built:
  • catppuccin-sddm

Copy link
Member

@keenanweaver keenanweaver left a comment

Choose a reason for hiding this comment

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

Working well for me. This would also close #279136

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/3669

@khaneliman
Copy link
Contributor

Working well for me. This would also close #279136

#255808 as well

pkgs/data/themes/catppuccin-sddm/default.nix Outdated Show resolved Hide resolved
pkgs/data/themes/catppuccin-sddm/default.nix Outdated Show resolved Hide resolved
maintainers/maintainer-list.nix Show resolved Hide resolved
@ElysaSrc ElysaSrc changed the title catppuccin-sddm: init at main-2024-03-17 catppuccin-sddm: init at 1.0.0 Mar 23, 2024
@ElysaSrc
Copy link
Contributor Author

Force pushed the branch:

  • Bumped to v1.0.0
  • Using --replace-fail
  • Cleaned up redundant substituteInPlace

@ElysaSrc
Copy link
Contributor Author

Is there any issue remaining preventing this PR to be merged ?

@ElysaSrc
Copy link
Contributor Author

Updated the branch, using kdePackages.callPackage instead of the libsForQt5.callPackage

Copy link
Member

@Aleksanaa Aleksanaa left a comment

Choose a reason for hiding this comment

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

You have to add runHook if you override phases. See https://nixos.org/manual/nixpkgs/unstable/#sec-stdenv-phases

Here are buildPhase and installPhase

@Sigmanificient
Copy link
Member

Can you re-fetch master and rebase? Eval is failing and was fixed by #305609.

@ElysaSrc
Copy link
Contributor Author

@Sigmanificient That should be ok

Copy link
Contributor

@pluiedev pluiedev left a comment

Choose a reason for hiding this comment

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

Shouldn't all new packages be using the pkgs/by-name convention, per RFC 140?

@ElysaSrc
Copy link
Contributor Author

@pluiedev I'm sorry, I checked the RFC but didn't understand what I'm supposed to do ? Could you point me in a direction ? Thanks !

@pluiedev
Copy link
Contributor

@pluiedev I'm sorry, I checked the RFC but didn't understand what I'm supposed to do ? Could you point me in a direction ? Thanks !

Move pkgs/data/themes/catppuccin-sddm/default.nix to pkgs/by-name/ca/catppuccin-sddm/package.nix, and remove the addition to pkgs/top-level/all-packages.nix

@ElysaSrc
Copy link
Contributor Author

@pluiedev I'm sorry, I checked the RFC but didn't understand what I'm supposed to do ? Could you point me in a direction ? Thanks !

Move pkgs/data/themes/catppuccin-sddm/default.nix to pkgs/by-name/ca/catppuccin-sddm/package.nix, and remove the addition to pkgs/top-level/all-packages.nix

Sorry for the delay, made the changes, rebased on more recent master.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/3917

@Aleksanaa Aleksanaa merged commit 48695bb into NixOS:master May 12, 2024
25 checks passed
@ribru17
Copy link

ribru17 commented May 16, 2024

Thank you so much for this! Just wanted to note that when using this theme the settings.Theme.CursorTheme option for sddm is now ignored for me. Must those values be set in catppuccin-sddm's sddm.conf?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet