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=736062 Nils Philippsen <nphilipp@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag| |needinfo?(karsten@xxxxxxxxx | |m) --- Comment #1 from Nils Philippsen <nphilipp@xxxxxxxxxx> 2011-09-07 06:31:50 EDT --- Items marked "GOOD" or "PASS" fulfil the guidelines or they don't apply to this package. Items marked "CHECK" aren't covered by the guidelines but you should check and fix them anyway in my opinion. Items marked "BAD" violate the guidelines in some point and need to be fixed. - GOOD/CHECK(?): rpmlint gives these warnings/errors on the source RPM (binary arches not checked because this package is exclusively for one secondary arch to which I don't have direct access): nils@gibraltar:~/devel/reviews/fedora/ppc64-diag> rpmlint ppc64-diag-2.4.2-1.src.rpm ppc64-diag.src: W: spelling-error %description -l en_US servicelog -> service log, service-log, serviceable ppc64-diag.src: W: spelling-error %description -l en_US config -> con fig, con-fig, configure ppc64-diag.src:43: E: hardcoded-library-path in $RPM_BUILD_ROOT/lib/systemd/system/ ppc64-diag.src:44: E: hardcoded-library-path in $RPM_BUILD_ROOT/lib/systemd/system/ ppc64-diag.src:69: E: hardcoded-library-path in /lib/systemd/system/ppc64-diag.service 1 packages and 0 specfiles checked; 3 errors, 2 warnings. -> please check if "servicelog" should rather be spelled "service log" (if it's a special PPC phrase it might be written together, but I don't know that) -> "config" is part of a file name mentioned in the description -- does that file need to be mentioned in the package description at all? -> /lib/systemd/system is the same on all architectures, but the %_unitdir macro should be used: http://fedoraproject.org/wiki/Packaging:Systemd#Filesystem_locations - GOOD/CHECK: named according to naming guidelines, but doesn't have a dist tag (cf. http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Using_the_.25.7B.3Fdist.7D_Tag). - GOOD: spec file name matches package name - GOOD/CHECK: licensed well (EPL), maybe ping upstream to clarify what is released under which license -- http://sourceforge.net/projects/linux-diag/ mentions that the software is licensed as "Eclipse Public License, GNU General Public License (GPL), GNU Library or Lesser General Public License (LGPL)" but this isn't mentioned anywhere in the tarball (it only has the EPL license) - GOOD: license field match actual license - GOOD: package ships license text as %doc - GOOD: package is written in American English - GOOD: spec file is legible - BAD: sources used to build the package must match the upstream source: upstream only has tarballs up to version 2.4.0: nils@gibraltar:~/devel/reviews/fedora/ppc64-diag> spectool -g ppc64-diag.spec Getting http://downloads.sourceforge.net/project/linux-diag/ppc64-diag/2.4.2/ppc64-diag-2.4.2.tar.gz to ./ppc64-diag-2.4.2.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 curl: (22) The requested URL returned error: 404 - PASS/CHECK: can't check if package successfully builds into binary package in mock (secondary arch only) - GOOD: package uses ExclusiveArch: to restrict it to the architectures for which it is intended - PASS/CHECK: can't check if it lists all build requirements - BAD/CHECK: the systemd service file should probably list the service type as "forking" - BAD/CHECK: use "make %{?_smp_mflags}" to build in parallel unless this package doesn't support parallel builds - PASS: no locale specific files - PASS: no shared library files included - GOOD: doesn't bundle system libraries - PASS: package not marked as relocatable - BAD: package must own all directories that it creates: %{_datadir}/ppc64-diag --> it'd probably be best/easiest to just list this directory in the file list so all stuff beneath it gets included recursively - GOOD: doesn't list files more than once - GOOD/CHECK: permissions on files set properly, but I find it strange that each file is listed with its own %attr() directive, I'd rather ensure that files get installed with correct permissions and get rid of the %attr()s - BAD: The package doesn't use macros consistently: /usr/libexec -> %_libexecdir /usr/sbin -> %_sbindir - GOOD: package contains code - PASS: no large documentation - GOOD: %doc doesn't affect runtime - PASS: no header files - PASS: no static libraries - PASS: doesn't contains libraries with suffix - PASS: no -devel subpackage - PASS: no libtool archive files on account of no libraries - PASS: no GUI app - GOOD: doesn't own files or directories owned by other packages - GOOD: all file names are valid UTF-8 - CHECK: rather than /usr/bin/yacc, /usr/sbin/lsvpd, require the byacc, lsvpd packages which contain them - CHECK: the dependency in /sbin/chkconfig seems bogus to me, the chkconfig patch unnecessary as the script isn't used as an init script anymore - CHECK: some scripts use perl, I'm not sure if this dependency is generated by rpmbuild or should be listed explicitly -- 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