[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries

[bugzilla at redhat.com]

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=694287

--- Comment #2 from Richard Shaw <hobbes1069 at gmail.com> 2011-04-07 09:58:13 EDT ---
(In reply to comment #1)
> 1/ Can you show the output of rpmlint on the src.rpm and the installed package.
rpmlint for SRPM:
openCOLLADA.src: W: spelling-error Summary(en_US) Collada -> Collard, Collate,
Collage
openCOLLADA.src: W: spelling-error %description -l en_US gtempaccount ->
unaccountable, accountable, accounting
openCOLLADA.src:118: W: mixed-use-of-spaces-and-tabs (spaces: line 8, tab: line
118)
openCOLLADA.src: W: invalid-url Source0: openCOLLADA-svn836.tar.bz2
1 packages and 0 specfiles checked; 0 errors, 4 warnings.

The installed package created a lot of warning that I didn't understand but no
errors. I'll attach the output instead of pasting it here.


> Please fix the empty spaces in the middle of the lines for the %description

Empty lines? Fixed.

> 2/ Please remove Everything that is not usefull in fedora. This reduce
> readability.
> (specially everything before the 'Name' field as this is not usefull in
> fedora).

Those macros are used to build the fake lib version. I can't remove them since
they are used later. What should we do here?


> 3/ The version field is wrong, please follow:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Version_and_Release

I was not sure how to handle this since it has no "version" in the traditional
sense. If I understand the guidelines correctly then this package will always
be at version "1.0" with the svn tag in the release?


> 4/ Uneeded BR: gcc-c++ - It is added by default, please remove.

Fixed.


> 5/ Buildroot is undeed nowdays:
> https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag

I saw this before but the spec templates still have it in there so the
guidelines conflict here. Now I know it's truly save to remove...

Isn't one of the other sections, %clean if I remember correctly also now not
needed?


> 6/ Remove spurious 'echo %{buildir}'

Fixed


> 7/ rpm -E %{?jobsflag} is empty. fedora uses %{?_smp_mflags} instead. Please
> remove the previous rpm macro.

Fixed


> 8/ Where does CHANGES.TXT come from ? Please add an url if possible
> -----------
> # copy CHANGES.txt
> cp %{S:1} ./ 
> -----------
> It's better to use install -p

It's the svn log which I guess the Suse creator/maintainer of this package
either likes or Suse requires. Do we need this? It appears to only be added to
the -devel package...


> 9/ About: 
> # Add some docs, need to fix eol encoding with dos2unix in some files
> -> Please move this in %prep section
> 
> 10/# Manual install ...
> Are you sure this cannot be installed with scons ? You should fix it within
> scons files then.

Not sure. I'll ask the Suse maintainer why he chose this route.


> Here a preliminary pass for the review.

Thanks!

-- 
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.