[Bug 711313] Review Request: wicd-kde - a Wicd client built on the KDE Development Platform

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=711313

--- Comment #32 from Kevin Kofler <kevin at tigcc.ticalc.org> 2011-11-08 11:38:50 EST ---
Re the Conflicts, I don't care either way. The files don't conflict but the
functionality does. But in any case, that is an issue with the wicd core and
not related to this review.


Unfortunately, I see some issues remaining with the specfile:

> BuildRoot:      %{_tmppath}/%{name}-%{version}

As Gregor already pointed out, please either remove this line or use one of the
lines from:
https://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag
(The RPM in current versions of Fedora ignores BuildRoot entirely.)

> BuildRequires:  qt-devel

Better use qt4-devel, which:
* also works on old versions where Qt 3 was the default,
* will also work on future versions where Qt 5 will be the default and
* allows specifying a version without an Epoch, i.e. these will work if some
future version of wicd-kde starts requiring Qt 4.8 for whatever reason:
BuildRequires: qt4-devel >= 4.8.0
or:
BuildRequires: qt-devel >= 1:4.8.0
but this will not:
BuildRequires: qt-devel >= 4.8.0
and the Epoch tends to get forgotten in such cases.

> BuildRequires:  kdebase-devel

Are you sure you want kdebase-devel? I think you want kdelibs-devel, or better
kdelibs4-devel (same as for Qt above).

> %make_install -C %{_target_platform}

The %make_install macro should not be used in Fedora (except for broken legacy
tarballs which do not support DESTDIR nor INSTALL_ROOT, but all CMake-based
projects support DESTDIR), please use:
make install/fast DESTDIR="%{buildroot}" -C %{_target_platform}
instead. (For non-CMake projects, use just "install" instead of "install/fast".
"install/fast" is a CMake target which skips the checks for whether everything
to be installed is already built. We already run "make" in %build, so we can
rely on everything already being built by the time we get to %install.)

And you should use either %{buildroot} or ${RPM_BUILD_ROOT} consistently rather
than mixing the two.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.