[Bug 225631] Merge Review: busybox

bugzilla at redhat.com bugzilla at redhat.com
Mon Aug 27 13:33:35 UTC 2007


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: busybox


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


pertusus at free.fr changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+




------- Additional Comments From pertusus at free.fr  2007-08-27 09:33 EST -------
The package is in a much better shape. The patches are now
readable, as the spec is. Well done. 

I still have some comments, but they are not blockers.

I spot some remnants from the past:
#SELINUX Patch

%ifarch ppc64
#%patch4 -b .ppc64 -p1
%endif

mkdir -p $RPM_BUILD_ROOT/%{_mandir}/man1


Maybe a comment explaining that the petitboot .config file comes
from a previous version so the depconfig file is recreated using 
make oldconfig non interactively may be added -- or something like 
this.

You could use %__cc instead of hardcoding gcc.

Using other optflags than RPM_OPT_FLAGS (like -Os) is not considered 
right by some reviewers. I personally don't care much, I guess
you have a valid reason to do so. You must add a comment, though:

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

Also the build doesn't show the options used during compilation. 
How can they be checked? This deserves a spec file comment.


Nothing is a blocker, except if the compile flags turn out to
be wrong.

APPROVED

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




More information about the package-review mailing list