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: iptables https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225906 kevin@xxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review- ------- Additional Comments From kevin@xxxxxxxxx 2007-02-13 22:40 EST ------- OK - Package meets naming and packaging guidelines OK - Spec file matches base package name. OK - Spec has consistant macro usage. OK - Meets Packaging Guidelines. OK - License (GPL) OK - License field in spec matches OK - License file included in package OK - Spec in American English OK - Spec is legible. OK - Sources match upstream md5sum: dd965bdacbb86ce2a6498829fddda6b7 iptables-1.3.7.tar.bz2 dd965bdacbb86ce2a6498829fddda6b7 iptables-1.3.7.tar.bz2.1 See below - BuildRequires correct See below - Package is relocatable and has a reason to be. See below - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. See below - Package has correct buildroot OK - Package is code or permissible content. OK - Packages %doc files don't affect runtime. OK - Headers/static libs in -devel subpackage. See below - Spec has needed ldconfig in post and postun See below - .so files in -devel subpackage. See below - -devel package Requires: %{name} = %{version}-%{release} OK - Package compiles and builds on at least one arch. OK - Package has no duplicate files in %files. OK - Package doesn't own any directories other packages own. OK - Package owns all the directories it creates. See below - No rpmlint output. OK - final provides and requires are sane: SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should function as described. See below - Should have subpackages require base package with fully versioned depend. See below - Should have dist tag OK - Should package latest version 21 outstanding bugs - check for outstanding bugs on package. Issues: 1. The Source URL is no longer correct. Suggest: http://www.netfilter.org/projects/iptables/files/iptables-1.3.7.tar.bz2 2. Can the Prefix: %{_prefix} be removed? Is there any reason this package needs to be relocatable? 3. Minor: The BuildRequires: /usr/bin/perl isn't required, as perl is part of the default build root. 4. Is the %defattr(-,root,root,0755) needed? or will %defattr(-,root,root,-) work? 5. Use the default correct buildroot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) 6. Any reason the package makes a static lib instead of a shared lib? Does anything use iptables-devel? Might be nice to remove the .a and make a shared lib instead. 7. The devel and ipv6 subpackages have Requires: %{name} = %{version} Suggest: change that to: Requires: %{name} = %{version}-%{release} 8. Our buddy rpmlint says: a) W: iptables summary-ended-with-dot Tools for managing Linux kernel packet filtering capabilities. Suggest: remove . at end of summary. b) E: iptables tag-not-utf8 %changelog Looks like the checkins from Trond Eivind Glomsr<F8>d <teg@xxxxxxxxxx> are not proper ut8f. Suggest: Change to utf8. c) W: iptables strange-permission iptables.init 0755 Suggest: change the source file permissions to 644 and install it as 755 only when installing? d) E: iptables non-utf8-spec-file iptables.spec The encoding of the entire spec is not utf8. Suggest: run iconv on it? e) E: iptables broken-syntax-in-scriptlet-requires Requires(post,postun): chkconfig There was a rpm bug that made that syntax not work right. Suggest: Change to: Requires(post): /sbin/chkconfig Requires(preun): /sbin/chkconfig f) W: iptables redundant-prefix-tag Suggest: remove prefix. g) W: iptables rpm-buildroot-usage %prep rm -rf %{buildroot} Suggest: remove rm in prep stage. It's not needed here. h) W: iptables macro-in-%changelog postun W: iptables macro-in-%changelog preun Suggest: Change any %macro names in changelog to be %%macro so rpm doesn't try and expand them. i) E: iptables no-cleaning-of-buildroot %install Suggest: add the rm -rf $RPM_BUILD_ROOT here. j) W: iptables conffile-without-noreplace-flag /etc/rc.d/init.d/iptables Suggest: No need to mark the init script as a config file. k) E: iptables non-readable /etc/sysconfig/iptables-config 0600 I guess this can be ignored. Not sure how much security it provides however. l) W: iptables service-default-enabled /etc/rc.d/init.d/iptables Normally this is a no-no, but in this case I think we do want it enabled by default. m) W: iptables no-reload-entry /etc/rc.d/init.d/iptables reload doesn't make sense here. n) W: iptables-devel spurious-executable-perm /usr/include/iptables.h W: iptables-devel spurious-executable-perm /usr/include/ip6tables.h W: iptables-devel spurious-executable-perm /usr/include/iptables_common.h Suggest: Those headers should be mode 644. o) E: iptables-ipv6 executable-marked-as-config-file /etc/rc.d/init.d/ip6tables See item l above... should probibly not be a config file. p) W: iptables-ipv6 service-default-enabled /etc/rc.d/init.d/ip6tables W: iptables-ipv6 no-reload-entry /etc/rc.d/init.d/ip6tables W: iptables-ipv6 incoherent-init-script-name ip6tables Ignore. 9. Minor: Consider adding a dist tag? 10. 21 outstanding bugs. None appear to be directly packaging related. However, there are some high priority ones and the secmark bug should would be very nice to solve before f7. -- 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