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

Remove enabling of logToFile for every display manager #50033

Merged
merged 1 commit into from Nov 13, 2018

Conversation

Gerschtli
Copy link
Contributor

Motivation for this change

I don't like to log simultaneously to a file and the journal and I can't understand why services.xserver.displayManager.job.logToFile is set to true for these three display managers.
It is inconsistent, that this option is only set for a subset of all display managers. To disable logToFile I have to use lib.mkForce which is ugly because I am resetting the value to the default.

Things done

I have removed all occurences of services.xserver.displayManager.job.logToFile = true; in every display manager config.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option 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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

@Gerschtli Gerschtli changed the title nixos/{lightdm,sddm,xpra}: remove enabling of logToFile Remove enabling of logToFile for every display manager Nov 9, 2018
@aanderse
Copy link
Member

Seems reasonable to me, though it might not hurt to mention this change in the 19.03 notes just in case?

@Gerschtli
Copy link
Contributor Author

Do you think the change in the release notes is okay? I'm not sure about my skills in the english language.. :D

@aanderse
Copy link
Member

Looking good. Given the mention in the manual I think this is pretty simple to merge into master. Just need to find someone to merge it now...

@infinisil
Copy link
Member

The first introduction of this was in 2e088aa#diff-12d91a9ce5bfb436f1bfb642126ea347R93 (later logsXsession has been renamed to logToFile). I can't find any reason this was done, so I think it's fine to merge this.

@infinisil infinisil merged commit de0763e into NixOS:master Nov 13, 2018
@Gerschtli Gerschtli deleted the remove/dm-log-to-file branch November 14, 2018 07:46
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

4 participants