Bug 209608 - Review Request: dwdiff - Front end to diff for comparing files on a word per word basis
Summary: Review Request: dwdiff - Front end to diff for comparing files on a word per ...
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Tomas Mraz
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Keywords:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-10-06 12:19 UTC by Jakub Hrozek
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2007-06-03 09:42:21 UTC
notting: fedora-cvs+


Attachments (Terms of Use)

Description Jakub Hrozek 2006-10-06 12:19:50 UTC
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.

Comment 1 Tomas Mraz 2006-10-06 18:36:01 UTC
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

Comment 2 Jakub Hrozek 2006-10-07 19:11:45 UTC
>> 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

Comment 3 Tomas Mraz 2006-10-09 14:16:00 UTC
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.


Comment 4 Jakub Hrozek 2006-10-16 11:44:57 UTC
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

Comment 5 Tomas Mraz 2006-10-16 13:17:58 UTC
OK, everything seems to be fine now. ACCEPTED


Comment 6 Kevin Fenzi 2007-06-03 04:18:26 UTC
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?

Comment 7 Jakub Hrozek 2007-06-03 09:42:21 UTC
Built in rawhide, in updates-testing for FC7. 


Note You need to log in before you can comment on or make changes to this bug.