Bug 226354 - Merge Review: radvd
Summary: Merge Review: radvd
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Martin Bacovsky
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 20:48 UTC by Nobody's working on this, feel free to take it
Modified: 2010-01-27 09:40 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-01-27 09:40:44 UTC
Type: ---
jima: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 20:48:01 UTC
Fedora Merge Review: radvd

http://cvs.fedora.redhat.com/viewcvs/devel/radvd/
Initial Owner: mbacovsk@redhat.com

Comment 1 Jima 2007-02-13 15:06:35 UTC
Picking up for review...

Comment 2 Jima 2007-02-13 16:04:30 UTC
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!

Comment 3 Martin Bacovsky 2007-02-15 08:36:51 UTC
I hope I fixed all issues stated above. I really appreciated jima's detailed
guidelines. Thanks a lot Jima. Current version is radvd-1.0-3.fc7.

Comment 4 Jima 2007-02-15 18:48:06 UTC
Sorry that took so long, I had to synch the latest devel tree to test the build
against.

My findings are that the following rpmlint messages remain:

E: radvd non-standard-uid /var/run/radvd radvd
E: radvd non-standard-gid /var/run/radvd radvd
W: radvd dangerous-command-in-%postun userdel

which, as said above, are acceptable.

Thanks for the cooperation; radvd is APPROVED.

Comment 5 Martin Nagy 2008-04-11 19:19:27 UTC
ping: Martin, shouldn't this bug be closed?

Comment 6 Jiri Skala 2010-01-25 15:22:39 UTC
fixed most of E and W complained by rpmlint.


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