Bug 606127

Summary: Review Request: colortool - useful tool for web-designers/graphic artists
Product: [Fedora] Fedora Reporter: Tobias Vogel <tobias.vogel>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: bugs.michael, fedora-package-review, martin.gieseking, notting
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-06-15 16:00:57 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 201449    

Description Tobias Vogel 2010-06-20 16:07:51 EDT
Spec URL: http://colortool.googlecode.com/files/colortool-0.99-20102006svn.fc13.spec
SRPM URL: http://colortool.googlecode.com/files/colortool-0.99-20102006svn.fc13.src.rpm
Description: converts between color models, chooses web-safe colors & generates color-charts
PLEASE NOTE: This is my first package and I am looking for a sponsor. Thanks!
Comment 1 Michael Schwendt 2010-06-22 14:34:34 EDT
Only a brief look at the spec:


> Release: 20102006svn%{?dist}

https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages


> Requires: qt    

https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires


> %description

The guidelines are not very specific, but you are supposed to construct _full sentences_ in the description. Would not be a blocker criterion, though.


> /usr/bin/colortool
> /usr/share/pixmaps/colortool.xpm
> /usr/share/applications/ColorTool.desktop
> /usr/share/applnk/Graphics/ColorTool.kdelnk
> /usr/share/colortool/translations/*

https://fedoraproject.org/wiki/Packaging:Guidelines#Macros
 and
https://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage


> /usr/share/colortool/translations/*

https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
https://fedoraproject.org/wiki/Packaging:UnownedDirectories
Comment 2 Tobias Vogel 2010-06-22 16:32:32 EDT
I am sorry for the incorrect build.
Hopefully, this one is better...

SPEC: http://colortool.googlecode.com/files/colortool-0-0.1.20102006svn0042.fc13.spec

SRPM: http://colortool.googlecode.com/files/colortool-0-0.1.20102006svn0042.fc13.src.rpm
Comment 3 Tobias Vogel 2010-06-24 10:44:21 EDT
I just updated the package; so the current file urls are:

SPEC:
http://colortool.googlecode.com/files/colortool-1.0-1.fc13.spec

SRPM:
http://colortool.googlecode.com/files/colortool-1.0-1.fc13.src.rpm

Thanks!
Comment 4 Martin Gieseking 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. :)
Comment 5 Jason Tibbitts 2010-11-23 15:01:08 EST
It's been several months since that commentary with no response from the submitter.  I will close this soon if there is no further progress.
Comment 6 Tobias Vogel 2010-12-26 01:38:07 EST
I am sorry for my inactivity for this bug. I learned a lot and adjusted my SPEC-file according to this advices.
Because I am currently working on a new version of my tool, I didn't post a newer SPEC-file so far, as dependencies are changing while I'm working on the new version.
I will be back soon, with the new version of this tool and an updated SPEC-file for both, this and the upcoming version.
Comment 7 Jason Tibbitts 2012-06-15 16:00:57 EDT
After another 1.5 years with no progress, I'll just close this out.  You can reopen in the future if you do come up with that new package.