faustlive: init at 2017-12-05 #45493
Conversation
|
Well, I think we need some hints of style here! Are you ready? :) |
|
@AndersonTorres I mean yeah, go for it. |
| stdenv.mkDerivation rec { | ||
| name = "faustlive-${version}"; | ||
| version = "2017-12-05"; | ||
| src = fetchgit { |
There was a problem hiding this comment.
Here I will suggest to use fetchFromGitHub. It is a less general function than fetchgit. It is always a better idea to use specialized fetchers, it reduces the "mental load".
You can look at this code as an example (yes, it's mine!):
nixpkgs/pkgs/development/compilers/jwasm/default.nix
Lines 9 to 14 in 4a553dc
| { stdenv | ||
| , fetchgit | ||
| , llvm | ||
| , gnumake |
There was a problem hiding this comment.
Wait. I am a little confused.
Isn't gnumake and gnused part of stdenv? If you called stdenv, automatically GNU Make is present!
Did you find some difficulty? The expression works without that?
There was a problem hiding this comment.
Sorry, that was a leftover from an earlier version of the expression.
| , gnused | ||
| , coreutils | ||
| , which | ||
| }: |
There was a problem hiding this comment.
Wow. So much lines.
Try to organize them, putting various parameters in a same line (always respecting the magic 80-characters limit).
Also, try to organize them by, what I would say?, logic. Libraries as (libjack2, alsaLib, qt48Full) grouped in a line, build tools (as llvm and gnumake) in other, auxiliary utilities (coreutils, with, gnused) in another one... You don't need to be overly accurate here, I think you get the general idea...
| }; | ||
|
|
||
| meta = with stdenv.lib; { | ||
| description = "FaustLive is a standalone just-in-time Faust compiler"; |
There was a problem hiding this comment.
No self-reference is needed in a small description. You can just rewrite it as
A standalone just-in-time Faust compiler
(No ending period, as you already did).
| sha256 = "0sw44yd9928rid9ib0b5mx2x129m7zljrayfm6jz6hrwdc5q3k9a"; | ||
| }; | ||
|
|
||
| meta = with stdenv.lib; { |
There was a problem hiding this comment.
meta is supposed to be the last thing in the derivation.
|
|
||
| meta = with stdenv.lib; { | ||
| description = "FaustLive is a standalone just-in-time Faust compiler"; | ||
| longDescription = '' |
There was a problem hiding this comment.
Rewrite that longDescription, always respecting the magic 80-characters limit, and using two chars as indent.
Indeed, this line is so long I need to use the horizontal scrolling bars of my browser.
Another example (mine, again!):
nixpkgs/pkgs/applications/virtualization/bochs/default.nix
Lines 115 to 120 in 71f899c
| makeFlags = [ "PREFIX=$(out)" ]; | ||
|
|
||
| buildPhase = '' | ||
| substituteInPlace Build/Linux/buildversion --replace "#!/bin/bash" "" |
There was a problem hiding this comment.
It can be rewritten as
preBuild = "patchShebangs Build/Linux/buildversion";
ThysBallard
force-pushed
the
master
branch
2 times, most recently
from
ad9e916
to
c2cc82a
Compare
|
@AndersonTorres Thanks for the tips. Let me know if these changes did the job :) |
| homepage = http://faust.grame.fr/; | ||
| license = licenses.gpl3; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Useless newline. It can be removed.
| }; | ||
|
|
||
| } | ||
|
|
There was a problem hiding this comment.
Double newline at the end of file. Leave only one here.
P.S.: there are some spurious whitespaces in your code. They pollute Git logs. Specifically, lines 3, 17 and 19 have endline whitespaces.
|
Hopefully everything is clean and tidy now :) |
|
I have tested it locally. It compíles and builds cleanly. If no one else objects, I will merge it tomorrow. |
|
@AndersonTorres thanks for all the help! This was my first pull request ever even though I've been programming for years so thanks for dealing with my crap |
Motivation for this change
Added FaustLive package for fast prototyping with FAUST programming language
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)