[Bug 606127] Review Request: colortool - useful tool for web-designers/graphic artists
bugzilla at redhat.com
bugzilla at redhat.com
Thu Aug 19 15:06:10 UTC 2010
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=606127
Martin Gieseking <martin.gieseking at uos.de> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |martin.gieseking at uos.de
--- Comment #4 from Martin Gieseking <martin.gieseking at uos.de> 2010-08-19 11:06:09 EDT ---
Hi Tobias,
here are some more quick comments on your spec file:
- The summary is too generic. It should tell what the program does, not who the
intended users are. :)
- Drop at least the revision number (-7) from BR: qt-devel >= 4.5.3-7
(probably, "4.5" instead of "4.5.3" is sufficient too)
- Add BR: desktop-file-utils.
- You can drop Requires: qt (it's automatically picked up as a dependency).
- The creation of the .desktop looks a bit complicated. I recommend to replace
it with a heredoc, e.g. like this:
cat <<EOT >%{name}.desktop
[Desktop Entry]
Name=ColorTool
Comment=Colorspace converter and color chooser
Exec=colortool
Icon=colortool
Terminal=false
Type=Application
Categories=GNOME;GTK;Graphics;
EOT
Also, move these lines to the %build section.
- In the %install section install the .desktop file with
desktop-file-install --dir=%{buildroot}%{_datadir}/applications %{name}.desktop
(the additional parameters given in your spec are not necessary)
- drop export BINDIR/DATADIR from %install (not required)
- the "install" line for the manpage can be shortened as follows:
install -m 644 -p colortool.1 %{buildroot}%{_mandir}/man1/
- Don't mix $RPM_BUILD_ROOT and %{buildroot}. Use only one of both.
- In %files, replace
%{_mandir}/man1/colortool.1.gz
with
%{_mandir}/man1/colortool.1* or %{_mandir}/man1/%{name}.1*
because you should not rely on a concrete compression format in the spec file
- Add a newline between the %changelog revisions to increase legibility.
That's all for now. :)
--
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.
More information about the package-review
mailing list