Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.

Bug 226538

Summary: Merge Review: wget
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Karsten Hopp <karsten>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: karsten, redhat-bugzilla, robin.norwood, ruben
Target Milestone: ---Keywords: Reopened
Target Release: ---Flags: ruben: fedora-review+
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-01-14 14:31:32 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:

Description Nobody's working on this, feel free to take it 2007-01-31 21:16:28 UTC
Fedora Merge Review: wget

http://cvs.fedora.redhat.com/viewcvs/devel/wget/
Initial Owner: karsten@redhat.com

Comment 1 Ruben Kerkhof 2007-02-04 12:19:48 UTC
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.


Comment 2 Robin Norwood 2007-02-04 17:38:31 UTC
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)



Comment 3 Karsten Hopp 2007-02-05 16:01:45 UTC
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 !

Comment 4 Karsten Hopp 2007-02-05 16:06:20 UTC
hmm, not sure if this should be closed, I'll reopen:
http://fedoraproject.org/wiki/WarrenTogami/ReviewWithFlags?highlight=%28Review%29

Comment 5 Ruben Kerkhof 2007-02-05 16:37:27 UTC
You're right, I didn't approve it yet :-)
I'll have another look tonight.

Comment 6 Karsten Hopp 2007-02-05 16:46:09 UTC
I've checked the changelog again, there was %xx in there. rpmint took that as a
macro. fixed in -14

Comment 7 Ruben Kerkhof 2007-02-05 18:42:00 UTC
Hi Karsten,

Thanks for fixing everything.
This package is approved. Please leave the ticket assigned to yourself.

Comment 8 Ville Skyttä 2007-02-05 20:13:07 UTC
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.

Comment 9 Karsten Hopp 2008-01-14 14:31:32 UTC
closing now as review is done