[Nix-dev] Re: [Nix-commits] SVN commit: nix - 14430 - MarcWeber - nixpkgs/trunk/pkgs/lib

[Eelco Dolstra]

Hi,

Marc Weber wrote:

Some style comments (I want to keep the modular lib functions clean and
maintainable):

> +  # this can help debug your code as well - designed to not produce thousands of lines
> +  traceWhatis = x : __trace (whatis x) x;

- The comment above "traceWhatis" doesn't actually explain what it does.

- It should be "whatIs" and "traceWhatIs".  But I prefer something like
"showType" and "traceType".

- Please don't use the __XXX builtins, they were never really intended to be
used directly.  Either say builtins.trace, or use a "with" or "inherit" to add
them to the scope.  See for instance the use of head/tail in lists.nix.

> +  traceMarked = str: x: __trace (str + (whatis x)) x;
> +  attrNamesToStr = a : lib.concatStringsSep "; " (map (x : "${x}=") (__attrNames a));
> +  whatis = x :
> +      if (__isAttrs x) then

There are a lot of redundant parentheses in your code, which makes it look too
much like Lisp :-)  The parentheses around an if condition are unnecessary.
Likewise, "str + (whatis x)" can just be "str + whatis x".

-- 
Eelco Dolstra | http://www.st.ewi.tudelft.nl/~dolstra/