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=459855 --- Comment #1 from Andreas Thienemann <andreas@xxxxxxxxx> 2008-10-20 13:50:42 EDT --- The package will need a bit more work before I can approve it. OK: source files match upstream: aefe6c6ddb46fb5eb5927974f0080518f63cf61a OK: dist tag is present. OK: build root is correct. OK: license field matches the actual license. OK: license is open source-compatible. GPLv2+ OK: license text included in package. OK: latest version is being packaged. OK: BuildRequires are proper. OK: compiler flags are appropriate. OK: %clean is present. OK: package builds in mock. OK: debuginfo package looks complete. OK: no shared libraries are added to the regular linker search paths. OK: doesn't own any directories it shouldn't. OK: no duplicates in %files. OK: file permissions are appropriate. 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: no headers. OK: no pkgconfig files. OK: no libtool .la droppings. OK: desktop files valid and installed properly not necessary. PASS: package meets naming and versioning guidelines. Did you consider naming ncidd differently? Maybe ncid-server to fit in with the ncid-client? NOK: specfile is properly named, is cleanly written and uses macros consistently. The .spec/srpm file is a bit messy: * Please do use full URLs for Source0. Instead of "Source0: %{name}-%{version}-src.tar.gz" please use "Source0: http://downloads.sourceforge.net/%{name}/%{name}-% {version}-src.tar.gz" * Please name your patches accordingly: ncid-0.71-no-default-runlevels.patch would be a good convention to follow. * Please start your patches with the id 0. * Trim the descriptions. They are way too long. More then 10 lines should never be necessary. * I'd prefer you to move all the %post scripts to the bottom of the .spec between %clean and %file. It's again a convention most packagers seem to follow and it makes reading .spec files so much easier. * Swap the /usr hardcode in the %build and %install part for %{_prefix} NOK: rpmlint is silent. ncid-client.i386: W: spurious-executable-perm /usr/share/doc/ncid-client-0.71/ncid-mythtv ncidd.i386: E: non-ghost-file /var/log/cidcall.log ncidd.i386: E: zero-length /var/log/cidcall.log ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/ncidd $prog ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/ncidd $prog ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/ncidd $prog ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/ncidsip $prog ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/ncidsip $prog ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/ncidsip $prog ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/sip2ncid $prog ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/sip2ncid $prog ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/sip2ncid $prog ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/yac2ncid $prog ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/yac2ncid $prog ncidd.i386: W: incoherent-subsys /etc/rc.d/init.d/yac2ncid $prog ncid-page.i386: W: no-documentation ncid-popup.i386: W: no-documentation ncid-samba.i386: W: no-documentation ncid-speak.i386: W: no-documentation 8 packages and 0 specfiles checked; 2 errors, 17 warnings. Please remove the executable bit from the mythtv initscript. You might wanna drop it completely though. Please consider dropping the cidcall.log file and %ghost it instead if it will be created by the daemon if non-existant. The incoherent-subsys warnings should be ignoreable, possible the same for the plugin subpackages. NOK: package installs properly. The ncid-page tool will not install, the Require for mail is invalid. [root@workstation ncid]# repoquery --whatprovides mail [root@workstation ncid]# Either use mailx or /bin/mail as a dep. PASS: scriptlets present and work okay. It would be great though if you could rework the initscripts to support the newer upstart syntax. NOK: final provides and requires are sane: Requires: /bin/bash /bin/sh config(ncid-client) = 0.71-1.fc9 config(ncidd) = 0.71-1.fc9 festival kdebase kdelibs kdemultimedia libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1) libc.so.6(GLIBC_2.3) libc.so.6(GLIBC_2.3.4) libc.so.6(GLIBC_2.4) libpcap.so.0.9 mail ncid-client = 0.71-1.fc9 ncid-speak = 0.71-1.fc9 perl(Data::Dumper) perl(Getopt::Long) perl(Getopt::Std) perl(IO::Socket::INET) perl(Net::Pcap) perl(Pod::Usage) perl(POSIX) perl(strict) perl(warnings) rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rtld(GNU_HASH) smbclient /usr/bin/perl Provides: config(ncid-client) = 0.71-1.fc9 ncid-client = 0.71-1.fc9 config(ncidd) = 0.71-1.fc9 ncidd = 0.71-1.fc9 ncid-debuginfo = 0.71-1.fc9 ncid-page = 0.71-1.fc9 ncid-popup = 0.71-1.fc9 ncid-samba = 0.71-1.fc9 ncid-speak = 0.71-1.fc9 Problem is the mail dependency again. NOK: owns the directories it creates. /etc/ncid /usr/share/ncid are shared by the packages but no package owns these directories. Another observation: Why did you remove the shebang line from the ncidrotate-script? If it's supposed to be executable, drop it in /usr/libexec/%{name}/. A general suggestions: I found upstream .spec files often too much work to adapt to the fedora packaging guidelines which is why I'm rolling my own usually. Might be wort considering the next time. :) -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review