Skip to content

audio services: use mkEnableOption#18524

Merged
fpletz merged 1 commit intoNixOS:masterfrom
langston-barrett:mkEnableOption
Sep 12, 2016
Merged

audio services: use mkEnableOption#18524
fpletz merged 1 commit intoNixOS:masterfrom
langston-barrett:mkEnableOption

Conversation

@langston-barrett
Copy link
Contributor

Motivation for this change

Many, many services use the following snippet:

      enable = mkOption {
        default = false;
        type = types.bool;
        description = ''
          Whether to enable Mopidy, a music player daemon.
        '';
      };

That's why mkEnableOption was created!

  mkEnableOption = name: mkOption {
    default = false;
    example = true;
    description = "Whether to enable ${name}.";
    type = lib.types.bool;
  };

KISS, DRY, etc. I wrote the following (unreadable) script:

#!/usr/bin/env nix-shell
#! nix-shell -i bash -p ag

cd "$1"

for file in $(IFS=\n ag -l "enable = mkOption"); do
  perl -0777 -p -i -e 's/(.+?)enable\s+=\s+mkOption\s+\{.+?to enable (.+?)\..+?};/\1enable = mkEnableOption "\2";/igs' "$file"
done

which I propose we use on everything in nixos/modules. I wanted to create this small PR first to discuss the possible implications of that massive change before actually implementing it.

We'd definitely need to be cautious about only using this function on services that are disabled by default.

What do y'all think?

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
    • OS X
    • Linux
  • 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.

@mention-bot
Copy link

@siddharthist, thanks for your PR! By analyzing the annotation information on this pull request, we identified @rickynils, @dmalikov and @shlevy to be potential reviewers

@fpletz fpletz added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Sep 12, 2016
@fpletz
Copy link
Member

fpletz commented Sep 12, 2016

We should definitely skim the diff, but that script should be very useful, thanks! 👍

Merging this as it won't be controversial. 😉

@fpletz fpletz merged commit 25a7ded into NixOS:master Sep 12, 2016
@langston-barrett langston-barrett deleted the mkEnableOption branch September 12, 2016 03:56
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants