[Bug 226354] Merge Review: radvd

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Merge Review: radvd


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226354


jima at beer.tclug.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|jima at beer.tclug.org         |mbacovsk at redhat.com
               Flag|fedora-review?              |fedora-review-




------- Additional Comments From jima at beer.tclug.org  2007-02-13 11:04 EST -------
OK - Spec in American English
OK - Spec is legible.
See below - Sources match upstream md5sum:
8bce4a21757cf069f5a69e2f9bee9e5b  radvd-1.0.tar.gz
8bce4a21757cf069f5a69e2f9bee9e5b  radvd-1.0.tar.gz.1
OK - BuildRequires correct
See below - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
See below - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
OK - final provides and requires are sane:

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
OK - Should function as described.
OK - Should have dist tag
OK - Should package latest version
0 bugs - check for outstanding bugs on package.

1. Source URL no longer correct.

Apparently you need to change the Source URL to:

http://www.litech.org/radvd/dist/radvd-%{version}.tar.gz

or such.  (Use of %{name} to replace one or more of the radvd's in there
is an option, as well.)

2. defattr:

The preferred format for defattr is now:

%defattr(-,root,root,-)

Not sure if it's a blocker or not.

3. BuildRoot:

The preferred format for BuildRoot is:

%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Probably not a blocker, but recommended.

4. rpmlint says:

on radvd-1.0-2.fc7.src.rpm:
W: radvd no-url-tag

Add "URL: http://www.litech.org/radvd/" or such.

W: radvd prereq-use chkconfig, /usr/sbin/useradd, /sbin/service, initscripts

According to rpmlint -I prereq-use:
The use of PreReq is deprecated. In the majority of cases, a plain Requires
is enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

W: radvd setup-not-quiet

Add -q to %setup line.

W: radvd macro-in-%changelog doc
W: radvd macro-in-%changelog config

I see most of the %'s in the changelog are properly escaped as %%, but
there are a couple that aren't.  Easy enough to fix.

on radvd-1.0-2.fc7.x86_64.rpm:
W: radvd no-url-tag

Addressed above...

W: radvd conffile-without-noreplace-flag /etc/rc.d/init.d/radvd
E: radvd executable-marked-as-config-file /etc/rc.d/init.d/radvd

%{initdir}/radvd really shouldn't be tagged %config; any configuration
options should be coming from /etc/sysconfig/radvd.

E: radvd non-standard-uid /var/run/radvd radvd
E: radvd non-standard-gid /var/run/radvd radvd

I suspect this is ignorable.

W: radvd dangerous-command-in-%postun userdel

Based on my inquiries, this is ignorable because radvd is found in
/usr/share/doc/setup*/uidgid .

5. %makeinstall

Usage of %makeinstall isn't recommended, see:

http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002

Not a blocker, but I suggest changing it.

Address the above, and we're probably good.  Thanks!

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.