Skip to content

xserver: dwm as a window manager#11415

Merged
jagajaga merged 1 commit intoNixOS:masterfrom
zenhack:dwm-wm
Dec 5, 2015
Merged

xserver: dwm as a window manager#11415
jagajaga merged 1 commit intoNixOS:masterfrom
zenhack:dwm-wm

Conversation

@zenhack
Copy link
Contributor

@zenhack zenhack commented Dec 2, 2015

No description provided.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @nbp, @AndersonTorres and @edolstra to be potential reviewers

Copy link
Member

Choose a reason for hiding this comment

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

Please use mkEnableOption.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quoting Arseniy Seroka (2015-12-03 08:27:59)

  • services.xserver.windowManager.dwm.enable = mkOption {

Please use mkEnableOption.

Can do. I based this off of another one of the files in that directory,
which still uses mkOption. I gather this is a deprecated way of doing
things that hasn't be cleaned up everywhere? Would a (separate) patch
that updates the rest be welcome?

Copy link
Member

Choose a reason for hiding this comment

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

Yes! That's going to be great work.
On 4 Dec 2015 01:47, "Ian Denhardt" notifications@github.com wrote:

In nixos/modules/services/x11/window-managers/dwm.nix
#11415 (comment):

+with lib;
+
+let
+

  • cfg = config.services.xserver.windowManager.dwm;

+in
+
+{
+

  • interface
  • options = {
  • services.xserver.windowManager.dwm.enable = mkOption {

Quoting Arseniy Seroka (2015-12-03 08:27:59)

  • services.xserver.windowManager.dwm.enable = mkOption { Please use
    mkEnableOption.
    Can do. I based this off of another one of the files in that directory,
    which still uses mkOption. I gather this is a deprecated way of doing
    things that hasn't be cleaned up everywhere? Would a (separate) patch that
    updates the rest be welcome?


Reply to this email directly or view it on GitHub
https://github.com/NixOS/nixpkgs/pull/11415/files#r46624887.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made the change you requested.

@AndersonTorres
Copy link
Member

Hum, so far so good. What about a fat commit changing all relevant files? I will do it now.

@zenhack
Copy link
Contributor Author

zenhack commented Dec 4, 2015

The travis failure seems to be unrelated to my change; error messages are talking about virt-manager and glance.

the mkEnableOption looks to be all over the source tree, not just in window-managers.

jagajaga added a commit that referenced this pull request Dec 5, 2015
xserver: dwm as a window manager
@jagajaga jagajaga merged commit 86c3f43 into NixOS:master Dec 5, 2015
@Janik-Haag Janik-Haag added the 12.first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

12.first-time contribution This PR is the author's first one; please be gentle!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants