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 #33 from Richard Shaw <hobbes1069@xxxxxxxxx> 2011-04-26 16:09:25 EDT --- (In reply to comment #28) > Needs work: > =========== > > Ok, I'm going to divide this into 2 lists, one with small things and one with > large things. First the small things: > - rpmlint checks return: > openCOLLADA.src:108: W: mixed-use-of-spaces-and-tabs (spaces: line 9, tab: > line 108) > Please don't use tabs in spec files. I couldn't find the tab but I ran across 'expand' which seemed to work quite nicely to fix the problem. > openCOLLADA.x86_64: W: file-not-utf8 /usr/share/doc/openCOLLADA-0/AUTHORS > You will need to convert this to utf8, add the following to %prep: > iconv -f ISO_8859-1 -t utf-8 COLLADAStreamWriter/AUTHORS > \ > COLLADAStreamWriter/AUTHORS.tmp > touch -r COLLADAStreamWriter/AUTHORS COLLADAStreamWriter/AUTHORS.tmp > mv COLLADAStreamWriter/AUTHORS.tmp COLLADAStreamWriter/AUTHORS Done. > - please drop the following comment (it is the same as Source0, so I see no > use for it): > #Source0: %{name}-svn%{AGE}.tar.xz Already gone. Sorry, I left that in there when I was playing with different compression (gz, xz, bz2) > - please drop the following (already commented) command from the spec file: > #strip --strip-debug *.so.* > Stripping should *never* be done inside the spec file. That's left over from Suse. It was always commented out for me. Fixed. > - please add -p to the invocation of cp for the doc files, to keep the original Fixed. > timestamps. Also why do you cp LICENSE, where as for AUTHORS you reference > it with its full path ? This seems inconsistent. Not sure, unless Dave just wanted AUTHORS in the subdir and LICENSE in the root. I don't see why AUTHORS couldn't be moved to the install root (is that the right terminology?) > - please add -k to the invocation of dos2unix to keep the original timestamps > on the files Done. > - Group: Applications/Productivity > Is not really appropriate, please use: > Group: System Environment/Libraries > For the main package instead. Apparently Suse has A LOT MORE groups than we do. I guessed on this one but that was before I really knew what I was doing :) > - # copy CHANGES.txt > install -p -m 0644 %{S:1} ./ > This belongs in %prep IMHO Done. > - Add -p to invocation of cp for installation of header files to preserve the > timestamps on the files This is already recursive. Would it be appropriate to just use -a here? > - devel does not requires base package n-v-r > Please change the: > Requires: %{name} = %{version} > For the -devel sub-package into: > Requires: %{name} = %{version}-%{release} Done. > - I don't like the way the changelog is formatted, here is a copy paste from > another review with similar issues: Done. Richard -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review