Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=469931 Terje Røsten <terjeros@xxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo? | --- Comment #28 from Terje Røsten <terjeros@xxxxxxxxxxxx> 2010-02-28 16:22:45 EST --- Thanks Andy much better, some comments: > Name: ipmiutil > Version: 2.6.0 > Release: 1 > Summary: A package that includes various IPMI server management utilities > License: BSD > Vendor: Kontron America, Inc. > Group: Applications/System > Source: http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz > URL: http://ipmiutil.sourceforge.net > BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root > BuildRequires: openssl-devel Move Summary: to the top, remove Vendor: line (it's added by the Fedora build system), add disttag (https://fedoraproject.org/wiki/Packaging:DistTag): Release: 1%{?dist} Change BuildRoot: to: BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) Optional: alignment of values. > %build > sh configure > make Add -j flags to make and use configure macro: %build %configure make %{?_smp_mflags} > %install > # %{buildroot} is set above in the header so it will never be "/" > # if [ "%{buildroot}" != "/" ] > rm -rf %{buildroot} Remove the comment. > export RPM_BUILD_DIR=`pwd` > export RPM_BUILD_ROOT=%{buildroot} > make install DESTDIR=%{buildroot} > # AFTER_INSTALL export stuff looks strange, really needed? Remove comment. > %clean > # %{buildroot} is set above in the header so it will never be "/" > rm -rf %{buildroot} Move this section to before %files section and remove comment > %files > %defattr(0755,root,root) Change to %defattr(- , root, root, -) > /etc/cron.daily/checksel Use sysconfdir for /etc > %defattr(0664,root,root) Remove. > %{_datadir}/%{name}/README > %{_datadir}/%{name}/COPYING > %{_datadir}/%{name}/UserGuide Remove too, add as %doc first in %files > %{_mandir}/man8/isel.8.gz > %{_mandir}/man8/isensor.8.gz Postfix might change to e.g. xz, change all mans to this style: %{_mandir}/man8/isensor.8* > # %defattr(-,root,root) Remove > # %doc README TODO COPYING ChangeLog Uncomment and move to top of %files, change to (add UserGuide here) %doc AUTHORS ChangeLog COPYING doc NEWS README TODO %doc doc/UserGuide What's left then are the scripts, looks much better, but still need clean up. A better aproach would be to move the logic to init scripts itself. The spec should only have ( https://fedoraproject.org/wiki/Packaging:SysVInitScript ): Requires(post): chkconfig Requires(preun): chkconfig # This is for /sbin/service Requires(preun): initscripts ... %post # This adds the proper /etc/rc*.d links for the script /sbin/chkconfig --add <script> %preun if [ $1 = 0 ] ; then /sbin/service <script> stop >/dev/null 2>&1 /sbin/chkconfig --del <script> fi We are making progress, please keep up the good work. Please include updated links to spec and srpm file when doing changes. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review