Spec URL: http://www.stud.fit.vutbr.cz/~xhroze01/dwdiff/dwdiff.spec SRPM URL: http://www.stud.fit.vutbr.cz/~xhroze01/dwdiff/dwdiff-1.2-1.src.rpm Description: dwdiff is a front-end for the diff program that operates at the word level instead of the line level. It is different from wdiff in that it allows the user to specify what should be considered whitespace, and in that it takes an optional list of characters that should be considered delimiters. Delimiters are single characters that are treated as if they are words, even when there is no whitespace separating them from preceding words or delimiters. Note that this my first package for Extras, therefore I need a sponsor.
GOOD: No rpmlint warnings or errors. BAD: Use full url on Source0 BAD: Use either $RPM_BUILD_ROOT + $RPM_OPT_FLAGS or the macros, not both. BAD: See packaging guidelines how to use %find_lang to package the localizations. It should Requires diffutils. It must BuildRequires at least gettext and diffutils (as diff is checked by the configure program) Next time read http://fedoraproject.org/wiki/Packaging/Guidelines
>> BAD: Use full url on Source0 >> BAD: Use either $RPM_BUILD_ROOT + $RPM_OPT_FLAGS or the macros, not both. >> BAD: See packaging guidelines how to use %find_lang to package the >> localizations. Thanks, fixed >> >> It should Requires diffutils. Fixed, I've ommited it because of http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions but you're right that that document mentions BuildRequires not Requires >> >> It must BuildRequires at least gettext and diffutils (as diff is checked by >> the configure program) You're right about gettext, sorry, fixed. I'm aware of the fact that the package uses diff but according to http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions there's no need to include diffutils in the BuildRequires section. >> >> Next time read http://fedoraproject.org/wiki/Packaging/Guidelines Thanks for reviewing the package! packages that should fix the above problems are located at: spec URL: http://www.stud.fit.vutbr.cz/~xhroze01/dwdiff/dwdiff.spec SRPM URL: http://www.stud.fit.vutbr.cz/~xhroze01/dwdiff/dwdiff-1.2-2.src.rpm
The %lang(nl) %{_mandir}/nl* in filelist will include the nl directories into the package which is wrong. Only the individual manpage files should be owned by the package. Also I think that the various encodings of the man page should be removed in %install and only the nl/man1/.... should be included in the filelist.
Thanks again for reviewing the package! New packages that should fix the above problems are located at: Spec URL: http://www.stud.fit.vutbr.cz/~xhroze01/dwdiff/dwdiff.spec SRPM URL: http://www.stud.fit.vutbr.cz/~xhroze01/dwdiff/dwdiff-1.2-3.src.rpm
OK, everything seems to be fine now. ACCEPTED
It looks like this package was imported long ago, but never built as far as I can see. Is there some problem with building it? Jakub: Can you build this package and then close this review request?
Built in rawhide, in updates-testing for FC7.