[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