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=523540 --- Comment #61 from Dominic Hopf <dmaphy@xxxxxxxxxxxxxxxxx> 2010-05-22 15:04:20 EDT --- Guys, thanks very much for your efforts. Here's another try of a formal review. There are at least two more small issues which need to be fixed (scroll down for those) before I will approve the package. $ rpmlint opentracker.spec opentracker.spec: W: invalid-url Source0: opentracker-0.tar.bz2 0 packages and 1 specfiles checked; 0 errors, 1 warnings. This warning can be ignored, since opentracker is rolling release. $ rpmlint opentracker-0-0.12.20100410cvs.fc12.src.rpm opentracker.src: W: invalid-url Source0: opentracker-0.tar.bz2 1 packages and 0 specfiles checked; 0 errors, 1 warnings. See above. $ rpmlint opentracker-debuginfo-0-0.12.20100410cvs.fc12.x86_64.rpm \ opentracker-ipv4-0-0.12.20100410cvs.fc12.x86_64.rpm \ opentracker-ipv6-0-0.12.20100410cvs.fc12.x86_64.rpm \ opentracker-rootdir-0-0.12.20100410cvs.fc12.noarch.rpm opentracker-ipv4.x86_64: W: spelling-error Summary(en_US) ipv -> iv, IPA, iPod opentracker-ipv4.x86_64: W: no-manual-page-for-binary opentracker-ipv4 opentracker-ipv6.x86_64: W: spelling-error Summary(en_US) ipv -> iv, IPA, iPod opentracker-ipv6.x86_64: W: no-manual-page-for-binary opentracker-ipv6 opentracker-rootdir.noarch: W: spelling-error Summary(en_US) dir -> deer, rid, Dir opentracker-rootdir.noarch: W: spelling-error %description -l en_US Filesystem -> File system, File-system, Systematic opentracker-rootdir.noarch: W: spelling-error %description -l en_US chroot -> cheroot, ch root, ch-root opentracker-rootdir.noarch: W: no-documentation opentracker-rootdir.noarch: W: non-standard-gid /var/opentracker opentracker 4 packages and 0 specfiles checked; 0 errors, 9 warnings. Manpages should be provided by upstream. Spelling issues can be ignored, I think the wording is okay. The other rootdir package warnings can also be ignored. Package Review ============== Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines [x] Specfile name matches %{name}.spec [x] Package seems to meet Packaging Guidelines [x] Package successfully compiles and builds into binary RPMs on at least one supported architecture. Tested on: Fedora 12/x86_64 [x] Rpmlint output: source RPM: see above binary RPM: see above [x] Package is not relocatable. [x] License in specfile matches actual License and meets Licensing Guidelines License: Beerware [-] License file is included in %doc. [x] Specfile is legible and written in AE [x] Sourcefile in the Package is the same as provided in the mentioned Source diff and meld do not show any differences between an own checkout and provided tarball [x] Package compiles successfully [x] All build dependencies are listed in BuildRequires [-] Specfile handles locales properly [-] ldconfig called in %post and %postun if required [x] Package owns directorys it creates [-] Package requires other packages for directories it uses. [x] Package does not list a file more than once in the %files listing [x] %files section includes %defattr and permissions are set properly [x] %clean section is there and contains rm -rf %{buildroot} [x] Macros are consistently used [x] Package contains code, or permissable content. [-] Large documentation files are in a -doc subpackage [x] Program runs properly without files listed in %doc [-] Header files are in a -devel package [-] Static libraries are in a -static package [-] Package requires pkgconfig if .pc files are present [-] .so-files are put into a -devel subpackage [-] Subpackages include fully versioned dependency for the base package [-] Any libtool archives (*.la) are removed [-] contains desktop file (%{name}.desktop) if it is a GUI application [x] Package does not own files or directories owned by other packages. [x] %{buildroot} is removed at beginning of %install [-] Filenames are encoded in UTF-8 === SUGGESTED ITEMS === [-] Package contains latest upstream version [x] Package does not include license text files separate from upstream. [-] non-English translations for description and summary [x] Package builds in mock Tested on: F12/x86_64 [x] Package should compile and build into binary RPMs on all supported architectures. tested build with koji [!] Program runs The opentracker-ipv6 service does not start. See below. [-] Scriptlets must be sane, if used. [-] pkgconfig (*.pc) files are placed in a -devel package [-] require package providing a file instead of the file itself no files outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin are required === Issues to point out === ==== 1 ==== The user change for the directory /var/%{name} doesn't seem to work as intended. The user opentracker is also needed for the opentracker-rootdir subpackage, but you create it just with the %pre ipv4 and %pre ipv6 commands. This results in a warning when installing the package with yum: Running Transaction Installing : opentracker-rootdir-0-0.12.20100410cvs.fc12.noarch 1/4 warning: group opentracker does not exist - using root /var/opentracker is not owned by the user opentracker then, but by the user root. Since the user creation is the same for both subpackages, I suggest to create the user once, when installing the rootdir subpackage (since it is installed as the first package). ==== 2 ==== I get an error when trying to start the opentracker-ipv6 service: # LANG=C service opentracker-ipv6 start opentracker-ipv6 starten: /etc/init.d/opentracker-ipv6: Zeile 36: -f: Kommando nicht gefunden. [FEHLGESCHLAGEN] I can not do any deeper testing for IPv6, unfortunately. ==== 3 ==== I think there is no need for two different configuration directories (when the ipv4 and ipv6 package is installed) in /etc/. It should be enough to have one directory (/etc/opentracker/) with two different files (opentracker-ipv4.conf, opentracker-ipv6.conf) in it. However, this one shall not block the review. -- 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