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@xxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|dcantrell@xxxxxxxxxx |pertusus@xxxxxxx Flag|fedora-review? |fedora-review+ ------- Additional Comments From pertusus@xxxxxxx 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review