Requesting review (II)/approval on Enlightenment DR17 package+ some EFLs

[Nils Philippsen]

On Fri, 2005-06-03 at 21:04 +0800, Didier Casse wrote:

> The spec files are here:

Where things apply to more than one spec file, please do it (I won't
repeat stuff for all packages, just for the first where I found it).

> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/eet.spec

- "Summary: Eet is a library for speedy data storage, retrieval, and
compression" -- Keep the summary as short as possiblr, don't repeat the
name in the summary, nor the article ("a") or punctuation, e.g.
"Summary: Library for speedy data storage, retrieval and compression".
Something like "Headers, static libraries, documentation and test
programs for %name development" is okay, though.

- "Release: 1.20050603.e17.fc3" -- do you really need to have the e17 in
the release?

- Consider including the CVS date in the packages' tarball names, not
only the version, e.g. like this:

...
%define cvsdate 20050603
...
Version: 0.9.10.008
Release: 1.%{cvsdate}.fc3
...
Source0: %{name}-%{version}-%{cvsdate}.tar.gz
...

- "%configure --prefix=%{_prefix} --bindir=%{_bindir} --libdir=
%{_libdir} --includedir=%{_includedir}" is redundant, "%configure" is
sufficient (try "rpm --eval %configure" to see what I mean)

- "%{__make} %{?_smp_mflags} %{?mflags}" -- I'd remove %{?mflags}, this
just smells like "non-repeatable build"

- "%{__make} %{?mflags_install} DESTDIR=%{buildroot} install", the same
as above, besides doesn't %makeinstall work?

- "%undefine __check_files" -- rather remove non-needed files in
%install

- "%{_libdir}/*.la" belongs into the devel subpackage (if at all)

> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/edb.spec
> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/embryo.spec
> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/evas.spec

- Don't do this:

CFLAGS="$RPM_OPT_FLAGS -fomit-frame-pointer -msse -mmmx -pipe"
export CFLAGS

This will generate an i386 package which doesn't run on machines up to
Pentium (Pro?), besides: -msse implies -mmmx. You can request a build
for i686 additionally to i386 which will pick up these flags via
$RPM_OPT_FLAGS _if_ such a package has that much improved performance
(please check this and come up with some numbers as this was deemed
worthwhile for kernel, glibc and openssl so far). It would be best if
upstream would build optimized versions of the performance-relevant
functions in all variants and determine processor capabilities at
startup, choosing the best-optimized functions _which work_ on the
processor at runtime. 

> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/ecore.spec

- I'd include $RPM_OPT_FLAGS in this one:

CFLAGS="-I/usr/kerberos/include -I/usr/X11R6/include/X11/extensions"

> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/edje.spec

This one is missing.

> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/epeg.spec
> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/epsilon.spec

- Full stop missing in %description

> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/etox.spec
> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/esmart.spec
> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/ewl.spec
> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/entrance.spec

- "%description 
Entrance  is the Enlightenment Display Manager. And like Enlightenment, 
it takes beauty and customization to levels that KDM and GDM can only 
dream about, without the bloat." -- Take out the last sentence, we don't
have GNOME packages bashing KDE in their %description or vice versa
either.

> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/enlightenment.spec

- Consider putting the session desktop file into the source RPM instead
of generating it in %install (there aren't any variables in it, so
there's no need to do that).

- "%{_libdir}/libe.*" -- is it correct that these are all in the devel
subpackage?

> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/engage.spec

- "Summary: Engage is a cool docker that resembles OSX in some ways" --
besides what I said above, don't include trademarks in the summary or
description.

- "Group: Applications/Multimedia" -- this is wrong, "User
Interface/Desktops" would fit IMO

> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/engrave.spec
> http://sps.nus.edu.sg/~didierbe/fedora/extras/specs/e17genmenu.spec

HTH,
Nils
-- 
     Nils Philippsen    /    Red Hat    /    nphilipp at redhat.com
"They that can give up essential liberty to obtain a little temporary
 safety deserve neither liberty nor safety."     -- B. Franklin, 1759
 PGP fingerprint:  C4A8 9474 5C4C ADE3 2B8F  656D 47D8 9B65 6951 3011