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

breeze-plymouth: add optional nixos branding, enable for plasma5 #31378

Merged
merged 2 commits into from Nov 18, 2017

Conversation

michaelpj
Copy link
Contributor

Motivation for this change

I noticed that the breeze-plymouth theme has some options for customizing it for a distro. I thought it would be nice to do this for nixos.

I then enabled this for the version that's used by plasma5 by default.

Note that we need to set some of the arguments in an override directly in the nixos module, since we want to include the vesion.

The result is now quite a nice splash screen which includes the proper nixos version and logo. There are also some options to set a color gradient in the background, but the selection of colors is quite limited and I wasn't sure any of it looked better than just black, so I left it.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@orivej
Copy link
Contributor

orivej commented Nov 10, 2017

Thank you! Looking at the implementation of the breeze theme, I can see why you didn't make the branded variant independent from the breeze theme even though on the surface there is only the spinner in common. In the end though it may be better to extract it into a separate repository (if the breeze theme is independent from breeze-text.so) to make it the default NixOS theme (not restricting it to KDE users).

However, the current label under the logo is not useful or pretty. For me it reads NixOS 18.03.git.db3d846 which makes it wider than the logo, but this git part of the version is just noise. I'd suggest either using NixOS for all versions, or NixOS for unstable and NixOS 17.09 for the current release. (When the version is empty the theme adds a space after the name, which moves it off center with the logo. I guess this may be counteracted by prepending a space to the name.)

There is also a tiny issue that both commits add some spaces at the ends of the lines. It is better to configure your editor to automatically trim whitespace.

@michaelpj
Copy link
Contributor Author

The problem with making it a separate derivation is that if you want to use the nixos branded one with a version, you need to be in a nixos context (like a module) where you have access to the version. So it seems like it has to be defined by an override in a module.

I did think about making it the default plymouth theme (IMO it's a lot nicer than the current default one), do you think I should just go ahead and do that?

I think I thought that the git suffix on the version was just if you'd built it from a git commit yourself, but I see that it's actually always there. In that case I agree, probably best to use the value of nixosRelease instead. I think it's not obvious when the user is on unstable - I could try and match on the string value of defaultChannel, but I'd rather not.

@michaelpj
Copy link
Contributor Author

Updated to use nixosRelease and strip whitespace.

I think I'll open a separate PR to suggest making this the default, and possibly also enabling it on the live CD to make a good first impression.

@michaelpj
Copy link
Contributor Author

Gentle ping @orivej . Are you happy with this now?

@orivej orivej merged commit 8991216 into NixOS:master Nov 18, 2017
@michaelpj
Copy link
Contributor Author

michaelpj commented Nov 18, 2017 via email

@orivej
Copy link
Contributor

orivej commented Nov 18, 2017

Thank you too!

@balsoft
Copy link
Member

balsoft commented Mar 22, 2018

This is probably for another issue, but when I enable this theme, my laptop hangs on boot to the point when it's completely unresponsive (ESC key doesn't work so I can't figure out what went wrong). If I hit ESC a bit earlier (before it hangs) and hide Plymouth, everything is fine.

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

5 participants