Re: [PATCH 1/5] gnu: Add efl.

[ludo@gnu.org (Ludovic Courtès)]

Tomáš Čech <sleep_walker@suse.cz> 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’.