Fedora Merge Review: wget http://cvs.fedora.redhat.com/viewcvs/devel/wget/ Initial Owner: karsten
Hi Karsten, Review for release 12: * RPM name is OK * Source wget-1.10.2.tar.gz is the same as upstream * Source wget-1.11-de.po is the same as upstream * This is the latest version * Builds fine in mock * File list looks OK * Config files of wget looks OK Needs work: * BuildRoot should be %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) (wiki: PackagingGuidelines#BuildRoot) * BuildRequires: perl should not be included (wiki: PackagingGuidelines#Exceptions) * Missing SMP flags. If it doesn't build with it, please add a comment (wiki: PackagingGuidelines#parallelmake) * The %makeinstall macro should not be used (wiki: PackagingGuidelines#MakeInstall) * The package should contain the text of the license (wiki: Packaging/ReviewGuidelines) COPYING is included in the source, just add it to %doc Notes: * Use Requires(post) and Requires(preun) instead of Prereq * Honour %{optflags} when building * Preserve timestamps when installing Rpmlint is not silent: Source RPM: W: wget summary-ended-with-dot A utility for retrieving files using the HTTP or FTP protocols. W: wget unversioned-explicit-provides webclient W: wget prereq-use /sbin/install-info W: wget macro-in-%changelog xx W: wget mixed-use-of-spaces-and-tabs (spaces: line 65, tab: line 48) rpmlint of wget: W: wget summary-ended-with-dot A utility for retrieving files using the HTTP or FTP protocols.
Also: o The unversioned-explicit-provides webclient is ok in this case - web browsers (mozilla, lynx, etc.) have this as well. o the prereq-use /sbin/install-info should be fixed as described here: - http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#info o I think: %find_lang %name Should be: %find_lang %{name} (consistent macro usage)
wget macro-in-%changelog xx is a false positive, usage of %%macroname is allowed. The rest should be fixed in wget-1.10.2-13.fc7. Thanks for the review !
hmm, not sure if this should be closed, I'll reopen: http://fedoraproject.org/wiki/WarrenTogami/ReviewWithFlags?highlight=%28Review%29
You're right, I didn't approve it yet :-) I'll have another look tonight.
I've checked the changelog again, there was %xx in there. rpmint took that as a macro. fixed in -14
Hi Karsten, Thanks for fixing everything. This package is approved. Please leave the ticket assigned to yourself.
rpmlint took %xx as a macro because it *is* a macro syntactically and even if it's not defined in clean buildroots, we don't know if someone has defined it locally.
closing now as review is done