[Nix-dev] Re: [Nix-commits] SVN commit: nix - r27769 - nixos/trunk/modules/services/logging

[Nicolas Pierron]

Hi Peter,

Thanks for working and contributing to this project.  I have a few
remarks to help you improve your next contributions on NixOS.

Types are good because they restrict the input of the option
definitions.  But other types annotations could be useful for you,
such as "" types.uniq "" or "" types.nullOr "", which can also be
combined.

* "" types.uniq t "" disallow more than one definition of type t.
This is useful when you can accept only one entry in a configuration
file and when there is no way to merge option definitions.

* "" types.nullOr t "" accept either a null value or any definitions
of type t.  This is useful when you may have an empty definition and
that any other value of the type may correspond to an acceptable
value.

tty option could have the type:  with types; uniq (nullOr string);
defaultConfig option could have type:  with types; uniq string;


On Wed, Jul 13, 2011 at 20:24, Peter Simons <simons at cryp.to> wrote:
> Modified: nixos/trunk/modules/services/logging/syslogd.nix
> ==============================================================================
> --- nixos/trunk/modules/services/logging/syslogd.nix    Wed Jul 13 18:15:57 2011        (r27768)
> +++ nixos/trunk/modules/services/logging/syslogd.nix    Wed Jul 13 18:24:53 2011        (r27769)
> @@ -19,44 +25,54 @@
>     *.crit                        /var/log/warn
>
>     *.*;mail.none;local1.none    -/var/log/messages
> -
> -    ${config.services.syslogd.extraConfig}
>   '';
> -
>  in
>
>  {
>   ###### interface
>
>   options = {
> -
> +
>     services.syslogd = {
>
>       tty = mkOption {
> +        type = types.string;
>         default = "tty10";
>         description = ''
>           The tty device on which syslogd will print important log
> -          messages.
> +          messages. Leave this option blank to disable tty logging.
> +        '';
> +      };
> +
> +      defaultConfig = mkOption {
> +        type = types.string;
> +        default = defaultConf;
> +        description = ''
> +          The default <filename>syslog.conf</filename> file configures a
> +          fairly standard setup of log files, which can be extended by
> +          means of <varname>extraConfig</varname>.
>         '';
>       };
>
>       extraConfig = mkOption {
> +        type = types.string;
>         default = "";
>         example = "news.* -/var/log/news";
>         description = ''
> -          Additional text appended to <filename>syslog.conf</filename>.
> +          Additional text appended to <filename>syslog.conf</filename>,
> +          i.e. the contents of <varname>defaultConfig</varname>.
>         '';
>       };
> -
> +
>     };
> -
> +
>   };
>
>
>   ###### implementation
>
>   config = {
> -
> +
>     jobs.syslogd =
>       { description = "Syslog daemon";
>


-- 
Nicolas Pierron
http://www.linkedin.com/in/nicolasbpierron - http://nbp.name/