[Bug 225691] Merge Review: dhcp

[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: dhcp


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


pertusus at free.fr changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|dcantrell at redhat.com        |pertusus at free.fr
               Flag|fedora-review?              |fedora-review+




------- Additional Comments From pertusus at free.fr  2007-04-11 15:10 EST -------
(In reply to comment #18)

> /var/run is documented in the LFS as the location for these files.  There should
> be no reason that we need rpmbuild to provide that information at build time.
> 
> Technically, I should be using /var/run/dhclient, but that's a patch for after
> Fedora 7.

I think that even if it is specified in the FHS, it is nice to
let a user rebuilding the package be able to specify completely
its install macros. 

Moreover if it is a patch that should be submitted upstream it is 
even more important to be able to specify another location
at build time. Not a blocker for the review in any case.


> > Issues:
> > W: dhcp strange-permission linux.dbus-example 0775
> > W: dhcp strange-permission dhcpd-conf-to-ldap 0775
> > W: dhcp strange-permission linux 0775
> > This should be fixed if possible.
> 
> Done.

No, it is not fixed, because it is the perms in the src.rpm,
but I think it cannot be fixed (because it is in cvs already).

> > If it is really the case, you can fix the following
> > W: dhcp mixed-use-of-spaces-and-tabs (spaces: line 11, tab: line 324)
> 
> I'm not seeing this.

In fact it is line 654 now, in the changelog:
- fix bug 176615: fix DDNS update when Windows-NT client sends
                  host-name with trailing nul
^^^^^^^^

> IMHO, all mention of dhcpcd should be removed from the spec file anyway.  That
> package hasn't been around for many years.

I think it is nice to keep Obsoletes/Provides for more than many 
years, when they are properly versionned. Still not a blocker, though. 
 

> >   - exit 0 is not very pretty
> 
> Not that it matters, but why is it not pretty?  It's pointless since the shell
> is already doing that, but why is it not pretty?

It is not pointless, it is certainly there to for an exit code
of 0 even if the preceding command had an exit code different from
0. I say it is not pretty because in my opinion it is better to 
avoid having an exit code of 1 in the first place.
 


Let's do a reduced formal review:

* rpmlint output ignorable (or not blocking)
W: dhcp invalid-license ISC
W: dhcp strange-permission linux.dbus-example 0775
W: dhcp strange-permission dhcpd-conf-to-ldap 0775
W: dhcp strange-permission linux 0775
E: dhcp configure-without-libdir-spec
W: dhcp mixed-use-of-spaces-and-tabs (spaces: line 11, tab: line 654)
W: dhcp invalid-license ISC
E: dhcp zero-length /var/lib/dhcpd/dhcpd.leases
W: dhclient invalid-license ISC
W: dhcp-devel invalid-license ISC
W: libdhcp4client invalid-license ISC
W: libdhcp4client no-documentation
W: libdhcp4client-devel invalid-license ISC
W: libdhcp4client-devel no-documentation
W: dhcp-debuginfo invalid-license ISC
* source match upstream, but source timestamp different.
  this should be fixed next time (by using spectoo -g, wget 
  -N or the like).
-rw-rw-r-- 1 dumas dumas 876591 nov  7 16:28 dhcp-3.0.5.tar.gz
-rw-rw-r-- 1 dumas dumas 876591 nov  6 00:01 dhcp-3.0.5.tar.gz-orig

ce5d30d4645e4eab1f54561b487d1ec7  dhcp-3.0.5.tar.gz

* follow guidelines
* specfile very legible and well commented
* scriptlets right
* %files section right


Minor:
according to the newest guideline, the *.a should be in 
a distinct package, named, for example 
dhcp-static-devel or the like, with 
Requires: %{name}-devel = %{epoch}:%{version}-%{release}

APPROVED

Reassigning to me since it should be assigned to reviewer.
Michael, you can reassign to you since you did the first review.

-- 
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.