> - It is not indented in the proper way (and it uses tabs...). For instance,
> the
> big "let localDefs" is indented, which it shouldn't be, and readability would
> be
> improved by separating the let-block from the surrounding code with empty
> lines.
OK, OK. Fixed it locally, will soon commit. I just dislike
spaces-instead-of-tabs.
I even didn't make a tab/space mix in the expression.
> - There is one gigantic doInstall phase. Why not use the generic builder in
> the
> right way? (Or is that because of texmfSrc?)
Because it was a work-in-progress. Or work in staggering and stumbling phase.
> - I don't understand all the strange "null" values, e.g. in "with builderDefs
> {src="";} null;" (and what does src="" do anyway?) and in "let localDefs =
> builderDefs (rec { ... }) args null; /* null is a terminator for sumArgs */".
OK, {src="";} refactored out. Actually, builderDefs defines some things that
are useful even while building its arguments, but I was not sure if it was
worth the effort to separate them (and have to think what will be useful and
what not). So I just created a skeleton builderDefs attrSet to use these
definitions. null is terminator for sumArgs, that is true - sumArgs takes a
function, then takes arbitrary number of attrSets to combine into arguments for
the function, and then does actual application when it gets null.
> - What does FullDepEntry do?
Composes a phase with non-trivial action and non-empty dependencies. You could
always spell out text= and deps=, but I find it less convenient.
> - Why is configureFlags set to [], but then configure is called with a long
> of
> arguments?
Experimental code and trivial to fix once it is relatively stable. Fixed.
> - Why does "meta" have "src" and "srcs" attributes?
I want to fetch only sources sometimes (as in I will leave soon, I will be
offline, but power is not a problem). So I want meta.src or just src to always
contain main source download, and meta.srcs would contain additional downloads.
I want to mark up new packages first. So I will once have enough packages to
force me to write expressions that actually automate the fetching.
> - For consistency, don't put a "." at the end of the description, and write
> it
> on one line, i.e.
>
> meta = {
> description = "A TeX distribution";
> };
>
> (BTW, "description" is intended for one-line descriptions on the Nixpkgs
> release page, but maybe we could also use an attribute for longer
> descriptions.)
OK.
> To reflect on the whole builderDefs / FullDepEntry / textClosure / null
> arguments style: it looks to me significantly more complicated than the old
> style of writing Nix expressions, compare e.g. the TeXLive expression to the
> teTeX expression
Not honest. teTeX build process is sane and polished. But generally, yes there
were too many things filled in from template.
> (https://svn.cs.uu.nl:12443/repos/trace/nixpkgs/trunk/pkgs/misc/tex/tetex/default.nix).
>
> There is a lot of duplicated code between expressions written in the style,
> which suggests that some abstraction is in order: if you find yourself
> writing
> (or cutting&pasting) the same code over and over, then it's time to put the
> commonality in a function...
OK.. First everything was very very unstable, so I tuned individual instances.
Now it seems to stabilize, so you are right and I actually have tried to move
everything in a function... If it survives rebuild, I will commit it.
_______________________________________________
nix-dev mailing list
[email protected]
https://mail.cs.uu.nl/mailman/listinfo/nix-dev
