I reviewed this package. The following issues need to be addressed for compilance with the guidelines:

* The files AUTHORS, ChangeLog, COPYING, HACKING, NEWS, README, TERMINOLOGY need to be listed under %doc

* The package owns
   %dir %{_datadir}/gimp
   %dir %{_datadir}/gimp/%{gimpsubver}
which are also owned by gimp. Note that the package Requires: gimp. The above ownerships need to be dropped.

* %defattr(-,root,root,-) is preferred. 

* BuildRequires: libxml2
is not required since
   BuildRequires: libxslt
will pull that up

* Similarly
   BuildRequires: docbook-style-xsl
is not required. It will be pulled up by other dependencies. Am I wrong?

ping?

Sorry for the late reply.

I've fixed the following in CVS, gimp-2.4.2-2 is building with these changes for Rawhide right now.

(In reply to comment #1)
> I reviewed this package. The following issues need to be addressed for
> compilance with the guidelines:
> 
> * The files AUTHORS, ChangeLog, COPYING, HACKING, NEWS, README, TERMINOLOGY
> need to be listed under %doc

These files get shipped except HACKING which is documentation about how to change the documentation for which you need the source anyway.

> * The package owns
>    %dir %{_datadir}/gimp
>    %dir %{_datadir}/gimp/%{gimpsubver}
> which are also owned by gimp. Note that the package Requires: gimp. The above
> ownerships need to be dropped.

Not owned anymore.

> * %defattr(-,root,root,-) is preferred. 

Fixed.

> * BuildRequires: libxml2
> is not required since
>    BuildRequires: libxslt
> will pull that up
> 
> * Similarly
>    BuildRequires: docbook-style-xsl
> is not required. It will be pulled up by other dependencies. Am I wrong?

The configure script of gimp-help checks directly for e.g. xmllint (included in libxml2) and .../xhtml/docbook.xsl which is included in docbook-style-xsl. I.e. both packages are required directly, therefore I keep listing them as build requirements. It doesn't matter that they in turn are required by other build requirements, I don't want to rely on that this fact doesn't change in the future.

Thanks for the update. There are two more little things:

* From the SPEC file:
    cat << EOF > files.list
    %%defattr (-, root, root, -)
    %%dir %{_datadir}/gimp/%{gimpsubver}/help
    %{_datadir}/gimp/%{gimpsubver}/help/images
    EOF
and
    echo "%%lang($dir) %{_datadir}/gimp/%{gimpsubver}/help/$dir" >> "$f"

The macro entries with single % expand into the files.list . This is equivalent to putting hard-coded paths into %files . I think those macro entries need double-% too.

* Parallel make must be supported whenever possible (If it is not supported, this should be noted in the SPEC file as a comment.). Sorry that I missed this in my initial review. Enabling the parallel make didn't cause a problem with my dual core. It certainly speeds things up.

(In reply to comment #4)
> Thanks for the update. There are two more little things:
> 
> * From the SPEC file:
>     cat << EOF > files.list
>     %%defattr (-, root, root, -)
>     %%dir %{_datadir}/gimp/%{gimpsubver}/help
>     %{_datadir}/gimp/%{gimpsubver}/help/images
>     EOF
> and
>     echo "%%lang($dir) %{_datadir}/gimp/%{gimpsubver}/help/$dir" >> "$f"
> 
> The macro entries with single % expand into the files.list . This is equivalent
> to putting hard-coded paths into %files . I think those macro entries need
> double-% too.

Technically they needn't be, as it doesn't really matter if they're expanded while files.list is built or while it is evaluated by rpmbuild. I've changed it to be more consistent, though.

> * Parallel make must be supported whenever possible (If it is not supported,
> this should be noted in the SPEC file as a comment.). Sorry that I missed this
> in my initial review. Enabling the parallel make didn't cause a problem with my
> dual core. It certainly speeds things up.

I've changed that and started building gimp-2.4.2-3.fc11 with these changes.

(In reply to comment #5)
> (In reply to comment #4)
> > The macro entries with single % expand into the files.list . This is equivalent
> > to putting hard-coded paths into %files . I think those macro entries need
> > double-% too.
> 
> Technically they needn't be, as it doesn't really matter if they're expanded
> while files.list is built or while it is evaluated by rpmbuild. I've changed it
> to be more consistent, though.
> 
You're right. I thought more about it. It shouldn't matter.

> > * Parallel make must be supported whenever possible (If it is not supported,
> > this should be noted in the SPEC file as a comment.). Sorry that I missed this
> > in my initial review. Enabling the parallel make didn't cause a problem with my
> > dual core. It certainly speeds things up.
> 
> I've changed that and started building gimp-2.4.2-3.fc11 with these changes.

I checked koji. It looks all good. 

-------------------------------------------
The package "gimp-help" is APPROVED by oget
-------------------------------------------

Package is in rawhide. We can close the bug now.