[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/5] gnu: Add efl.
From: |
Ludovic Courtès |
Subject: |
Re: [PATCH 1/5] gnu: Add efl. |
Date: |
Fri, 27 Feb 2015 17:56:28 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.4 (gnu/linux) |
Tomáš Čech <address@hidden> skribis:
> * gnu/packages/efl.scm: New file
> * gnu-system.am (GNU_SYSTEM_MODULES): Add it.
Nice work!
> + (uri (string-append
> "http://download.enlightenment.org/rel/libs/efl/efl-" version ".tar.gz"))
Please keep lines below 80 chars.
> + (inputs
> + `(("alsa-lib" ,alsa-lib)
> + ("bash" ,bash)
Adding Bash is most likely unnecessary, or if it is, you’ll have to add
a comment to justify it. :-)
> + (propagated-inputs
> + `(("bullet" ,bullet)
> + ("dbus" ,dbus)
> + ("eudev" ,eudev)
> + ("fontconfig" ,fontconfig)
> + ("freetype" ,freetype)
> + ("fribidi" ,fribidi)
> + ("glib" ,glib)
> + ("libpng" ,libpng)
> + ("libsndfile" ,libsndfile)
> + ("luajit" ,luajit)
> + ("openssl" ,openssl)
> + ("pulseaudio" ,pulseaudio)
> + ("util-linux" ,util-linux)))
That’s a lot! Normally we put a comment to justify why an input is
propagated. The usual reasons are: installed headers include headers of
a propagated input, or .pc file refers to one of them.
Could you double-check whether all these are needed, and add comments?
> + #:phases
> + (alist-cons-before
> + 'configure 'patch-config-files
> + (lambda _
> + (substitute* "po/Makefile.in.in"
> + (("/bin/sh") (which "bash"))))
> + %standard-phases)))
po/Makefile.in.in is not used, so I believe this phase is not needed.
> + (home-page "http://www.enlightenment.org")
> + (synopsis "Enlightenment Foundation Libraries")
> + (description
> + "EFL is toolkit used mainly for Enlightenment, but is used for more
> applications because it is resource friendly and energy efficient.")
“Toolkit” is vague; could you say a couple of words about what it
provides? My understanding is that it’s partly a graphical toolkit, but
also partly a library of data structures, file system convenience
functions, and such, right?
Also, s/Enlightenment/the Enlightenment desktop environment/ or
something like that.
> + (license (list license:bsd-2 license:lgpl2.1 license:zlib)))) ;
> different parts under different licenses
Please move the comment above to keep the lines below 80 chars.
Thanks!
Ludo’.
- Re: [PATCH 4/5] gnu: Add emotion-generic-players, (continued)
Re: [PATCH 4/5] gnu: Add emotion-generic-players, Ludovic Courtès, 2015/02/27
[PATCH 5/5] gnu: Add terminology., Tomáš Čech, 2015/02/25
Re: [PATCH 1/5] gnu: Add efl., Andreas Enge, 2015/02/26
Re: [PATCH 1/5] gnu: Add efl., Andreas Enge, 2015/02/26
Re: [PATCH 1/5] gnu: Add efl.,
Ludovic Courtès <=