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(a)free.fr changed:
What |Removed |Added
----------------------------------------------------------------------------
Flag|fedora-review+ |fedora-review?
------- Additional Comments From pertusus(a)free.fr 2007-02-27 19:22 EST -------
This package cannot be accepted as is; there are still must
fix issues. rpmlint is not silent. And overall many other things
to check.
Here we go:
* there is a huge pile of patches. Can't some of them be submitted
upstream? I have some remarks on some of them:
- dhcp-3.0.5-ldap-configuration.patch: is the last chunk really
needed? Is this patch from fedora or was it found somwhere else?
and has it been submitted upstream?
- dhcp-3.0.5-client.patch: the comment should explain what this patch
is about. There seems to be very specific fedora things mixed up
with new features, and these new features seem to be diverse. Maybe
this patch could be split?
- first chunk of dhcp-3.0.5-common.patch is very dubious (there are other
places with similar dubious code).
Most of the patch corrects compile-time warning, but there is also
a patch for a man page describing new features. Shouldn't this part
be split out and put in the patch where the features are added?
- dhcp-3.0.5-dhcpctl.patch dhcp-3.0.5-dst.patch,
dhcp-3.0.5-fix-warnings.patch, dhcp-3.0.5-minires.patch
dhcp-3.0.5-omapip.patch are mostly build fixes. Shouldn't those
patches be grouped together?
- dhcp-3.0.5-extended-new-option-info.patch has a new script in the
beginning. Is it really clean to have it mixed with the remaining?
- dhcp-3.0.5-includes.patch mixes build fixes, and different new
features (seems that some are in extended-new-option-info, others
in dhcp-3.0.5-client.patch
- libdhcp4client and timeouts patches seems clean to me, although it
seems to me that they should be merged. Has them been
submitted upstream?
- dhcp-3.0.5-server.patch seems clean too me, but there should be
a comment describing what is done in this patch. Has this been
submitted upstream?
To summarize it seems to me that the patches should be grouped such
that each new feature is in one patch and there is a patch containing
all the build fixes.
* Regarding the patch dhcp-3.0.5-Makefile.patch, wouldn't it be
better to override the LFLAGS make variable instead of patching?
And why are all those link flags added?
What is the aim of the dhcp-3.0.5/minires/Makefile.dist patch?
Is libdst really needed by something?
The changelog mentions bugs I am not allowed to view for those 2
items...
* setting RPM_OPT_FLAGS to RPM_OPT_FLAGS with other options is
ugly. Don't do that spec, won't be legible.
* there are many places where rpm macros should be used, for
sysconfdir and also others. You should also make sure that
in the code sysconfdir value is used.
* use $RPM_BUILD_ROOT or %{buildroot} consistently
* keep timestamps for data files, even those shipped in the srpm
* the ln -s in scriptlet for man pages seems wrong. What is it for?
* libdhcp4client-devel should Requires: pkgconfig
* Why is -fvisibility=hidden used by default?
* In my opinion
/etc/rc.d/init.d/*
shouldn't be marked as %config
* dhcp should not depend on perl
* -devel should requires main package with version-release
Suggestion:
use %defattr(-,root,root,-)
I reset the flag to ?
--
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.