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=753354 --- Comment #8 from Ondrej Vasik <ovasik@xxxxxxxxxx> 2012-01-20 05:58:10 EST --- formal review is here, see the notes explaining OK* and BAD (BAD*) statuses below: OK source files match upstream: sha256sum strongswan-4.6.1.tar.gz : d750ec16bc32c3d7f41fdbc7ac376defb1acde9f4d95d32052cdb15488ca3c34 strongswan-4.6.1.tar.gz One comment here - is there any reason for shipping .tar.gz sources? I see upstream provides .tar.bz2 as well, which is about 30% smaller. sha256sum strongswan-4.6.1.tar.bz2 : 3d6dcdb3ce46dab51783b98c9bb54ebc931ff80941a0507d3cf3e3ac813eb439 strongswan-4.6.1.tar.bz2 OK package meets naming and versioning guidelines. OK specfile is properly named, is cleanly written and uses macros consistently. OK dist tag is present. BAD license field matches the actual license. Still GPL - You should use GPLv2+ BAD license is open source-compatible (GPLv2+). License text included in package. at least %doc COPYING README is missing (maybe even NEWS could be helpful) OK latest version is being packaged. Only developer version for 4.6.2 available atm. OK BuildRequires are proper. OK compiler flags are appropriate. OK package builds in mock (Rawhide/i386). OK debuginfo package looks complete. BAD* rpmlint is silent. OK final provides and requires look sane. N/A %check is present and all tests pass. BAD shared libraries are added to the regular linker search paths with correct scriptlets %post -p /sbin/ldconfig %postun -p /sbin/ldconfig is missing ... package adds some shared libraries ... Idea - would it make sense to move them into -libs subpackage? BAD owns the directories it creates. /usr/libexec/ipsec/ , /usr/lib/ipsec/ , /usr/lib/ipsec/plugins directories are added but not owned. OK doesn't own any directories it shouldn't. OK no duplicates in %files. OK file permissions are appropriate. OK correct scriptlets present. OK code, not content. OK documentation is small, so no -docs subpackage is necessary. OK %docs are not necessary for the proper functioning of the package. OK headers in devel subpackage OK pkgconfig files in devel subpackage. OK no libtool .la droppings. OK not a GUI app. rpmlint output: strongswan.i686: W: invalid-license GPL Use GPLv2+ strongswan.i686: E: non-readable /etc/strongswan.conf 0640L Really strange permissions for the configure file... Is that intentional? I would recommend changing to 644... strongswan.i686: W: manual-page-warning /usr/share/man/man5/strongswan.conf.5.gz 27: warning: `EX' not defined strongswan.i686: W: manual-page-warning /usr/share/man/man5/strongswan.conf.5.gz 31: warning: `EE' not defined strongswan.i686: W: manual-page-warning /usr/share/man/man5/strongswan.conf.5.gz 141: warning: `TQ' not defined strongswan.i686: W: manual-page-warning /usr/share/man/man5/strongswan.conf.5.gz 1274: warning: `UR' not defined strongswan.i686: W: manual-page-warning /usr/share/man/man5/strongswan.conf.5.gz 1276: warning: `UE' not defined strongswan.i686: W: manual-page-warning /usr/share/man/man5/ipsec.conf.5.gz 1370: warning: `EX' not defined strongswan.i686: W: manual-page-warning /usr/share/man/man5/ipsec.conf.5.gz 1372: warning: `EE' not defined strongswan.i686: W: manual-page-warning /usr/share/man/man5/ipsec.secrets.5.gz 135: warning: `TQ' not defined Please check the manpages, it seems they are not completely gramatically correct. strongswan.i686: W: no-manual-page-for-binary strongswan I guess this is caused by incomplete renaming binary from ipsec to strongswan... please adjust the fix... 1 packages and 0 specfiles checked; 1 errors, 13 warnings. Small comments: There are trailing spaces in the spec file in %post section When building on RHEL it would emit also warning about empty sections of post/postun ... but this is something what could be solved later (you want this package in epel6, so it would probably be better if you do that soon) Please fix the issues. -- 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