Note: This is my first Fedora package.  I will need a sponsor.

Hi Michael,

here are some initial comments:

- according to the source file headers, the license seems to be GPLv2+ 
 (because of the addition "or (at your option) any later version")
  This must be reflected in the License field

- you can drop Requires: gd since it's picked up automatically as a dependency

- I suggest to run the tests coming with the tarball
  (add a %check section containing "make check")

- please use macro %{_defaultdocdir} instead of %{_docdir}

- in the %files section, replace the four %doc lines with 
  %{_defaultdocdir}/%{name}-%{version}/

- replace %{_mandir}/man1/mscgen.1.gz with %{_mandir}/man1/mscgen.1*
  (because the compression method applied by rpmbuild might change)

- In the %changelog section, you should only list changes made to the
  package/spec. Don't add the program's (upstream) changelog.

Thanks for the quick feedback, and apologies for my errors.

I've updated the spec file with your comments and made a new src package.  Please see the following locations:

Spec URL: http://www.mcternan.me.uk/mscgen/fc12/mscgen.spec
SRPM URL: http://www.mcternan.me.uk/mscgen/fc12/mscgen-0.18-2.fc12.src.rpm

There's no need to apologize. Actually, the purpose of the reviewing process is to find packaging issues and to fix them. It would be boring here if nobody made mistakes any longer. :)

An additional note:
Please remove the "%doc" prefix from %doc %{_defaultdocdir}/%{name}-%{version}/
For further details see
http://fedoraproject.org/wiki/Packaging_tricks#Installing_documentation:_2_paths

Okay - cool.  I've fixed the %doc part:

Spec URL: http://www.mcternan.me.uk/mscgen/fc12/mscgen.spec
SRPM URL: http://www.mcternan.me.uk/mscgen/fc12/mscgen-0.18-3.fc12.src.rpm

I read the packaging tricks page again, and *think* that's the only one from there...

OK, the spec file looks fine now. However, the source tarball in your SRPM doesn't match the upstream tarball:

$ md5sum mscgen-src-0.18.tar.gz*
2b6cfbabbc8bbc25eb52684410705510  mscgen-src-0.18.tar.gz
0922258a7fb86612bb623c1101260fd0  mscgen-src-0.18.tar.gz.1

You must use the untouched upstream archive to build the package. If it's necessary to modify parts of the sources, add patches to your package.

There's a short delay here while I fix upstream.  Then I'll make a new package (likely 0.19-1) with matching MD5s and retry.

OK, thanks for the feedback. No need to hurry.

> %files
> %defattr(-,root,root,-)
> %{_defaultdocdir}/%{name}-%{version}/

As %files sections like this keep popping up lately, note that explicitly including files in %_defaultdocdir conflicts with using %doc. It's advised to add a comment about that. For example,

 | %files
 | %defattr(-,root,root,-)
 | %{_defaultdocdir}/%{name}-%{version}/
 | %doc TODO

with Fedora 14 would give an error, whereas with older dists it might empty the contents of %_defaultdocdir and replace them with the %doc files without any warning.

%files
%defattr(-,root,root,-)
# due to this entry, %doc must not be used to add any other files
%{_defaultdocdir}/%{name}-%{version}/

Version 0.19 of mscgen is now out.  I've updated the spec file and added a warning comment as per Michael Schwendt's comment.  I also checked that the MD5 of the source package matches upstream.

New versions are as follows:

Spec URL: http://www.mcternan.me.uk/mscgen/fc13/mscgen.spec
SRPM URL: http://www.mcternan.me.uk/mscgen/fc13/mscgen-0.19-2.fc13.src.rpm

Michael, have you already done some informal reviews of other packager's submissions? If not, I'd encourage you to do so in order to show your understanding of the packaging guidelines. This is an important part of the sponsoring process. For further information have a look at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

I've looked at other packages to see how they fit together, particularly gd2.

I take your point though; I'll see if I can usefully comment on some other packages up for review.

Thanks to all that contributed to this review but I've decided not to pursue this any more.

Instead I've created my own repo for mscgen on Fedora and EPEL: 

  http://www.mcternan.me.uk/mscgen/yum/

If someone wished to package mscgen for Fedora directly I would be happy and support them, but using my own repo is easier for me.

*** This bug has been marked as a duplicate of bug 842064 ***