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

Redshift add early startup option, rewrite to use toINI and add more options #57975

Closed
wants to merge 2 commits into from

Conversation

xaverdh
Copy link
Contributor

@xaverdh xaverdh commented Mar 20, 2019

Motivation for this change

This adds an earlyStartup option, which allows to start redshift before the graphical interface is up.
While doing this, I realized that this can be done a lot simpler using toINI.

This is meant to solve #57961

@xaverdh xaverdh requested a review from infinisil as a code owner March 20, 2019 16:30
@GrahamcOfBorg GrahamcOfBorg added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Mar 20, 2019
@xaverdh xaverdh force-pushed the redshift-early-startup branch 6 times, most recently from 50ea77f to 9666364 Compare March 20, 2019 20:27
@xaverdh xaverdh changed the title WIP: Redshift add early startup option, rewrite to use toINI Redshift add early startup option, rewrite to use toINI and add more options Mar 20, 2019
@xaverdh xaverdh force-pushed the redshift-early-startup branch from 516be57 to e5f880b Compare March 24, 2019 00:02
@xaverdh xaverdh force-pushed the redshift-early-startup branch from 257616b to 81e39a1 Compare March 25, 2019 16:30
@ivan
Copy link
Member

ivan commented Sep 6, 2019

For the author, reviewers, and committers: this PR was scanned and appears to add a use of the deprecated types.string, which emits a warning as of #66346. Before merging, please change this to another type, possibly:

  • types.str for a single string where merging does not make sense, or cannot work
  • types.lines for multi-line configuration or scripts where merging is possible
  • types.listOf types.str for a mergeable list of strings

@@ -94,6 +123,15 @@ in {
'';
};

extraConfig = mkOption {
type = types.attrsOf types.string;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
type = types.attrsOf types.string;
type = types.attrsOf types.lines;

@infinisil
Copy link
Member

I think we can close this for #50979, which is being worked on again and overall more finished

@infinisil infinisil closed this Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants