Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 606127 - Review Request: colortool - useful tool for web-designers/graphic artists
Summary: Review Request: colortool - useful tool for web-designers/graphic artists
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2010-06-20 20:07 UTC by Tobias Vogel
Modified: 2012-06-15 20:00 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-06-15 20:00:57 UTC


Attachments (Terms of Use)

Description Tobias Vogel 2010-06-20 20:07:51 UTC
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 18:34:34 UTC
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 20:32:32 UTC
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 14:44:21 UTC
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 15:06:09 UTC
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 20:01:08 UTC
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 06:38:07 UTC
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 20:00:57 UTC
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.


Note You need to log in before you can comment on or make changes to this bug.