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=441570 --- Comment #20 from Kevin Fenzi <kevin@xxxxxxxxx> 2008-08-23 19:15:35 EDT --- Hey Jeff. Sorry for the long delay here... lets get this moving again with a formal review: 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 (GPLv2) 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: b8e262dd148f22d000015e638ab75b7e dnx-0.18.tar.gz b8e262dd148f22d000015e638ab75b7e dnx-0.18.tar.gz.orig OK - BuildRequires correct OK - Package has %defattr and permissions on files is good. OK - Package has a correct %clean section. OK - Package has correct buildroot %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) OK - Package is code or permissible content. OK - Doc subpackage needed/used. OK - Packages %doc files don't affect runtime. see below - Package has rm -rf RPM_BUILD_ROOT at top of %install OK - Package compiles and builds on at least one arch. See below - Package has no duplicate files in %files. See below - Package doesn't own any directories other packages own. See below - Package owns all the directories it creates. See below - No rpmlint output. See below - final provides and requires are sane: SHOULD Items: OK - Should build in mock. OK - Should build on all supported archs OK - Should have dist tag OK - Should package latest version Issues: 1. There shouldn't be any need for rm -rf %{buildroot} >/dev/null 2>&1 at the top of %install... should just be rm -rf %{buildroot} (there shouldn't be any output from that ever. 2. rpmlint says: dnx.src:142: E: files-attr-not-set dnx.src:143: E: files-attr-not-set I think this is due to the doc subpackage missing a: %defattr(-,root,root,-) line dnx.x86_64: W: non-standard-uid /var/run/dnx nagios dnx.x86_64: W: non-standard-gid /var/run/dnx nagios dnx.x86_64: W: non-standard-uid /var/log/dnx nagios dnx.x86_64: W: non-standard-gid /var/log/dnx nagios dnx-server.x86_64: W: non-standard-uid /var/log/dnx nagios dnx-server.x86_64: W: non-standard-gid /var/log/dnx nagios Ignore. dnx.x86_64: W: log-files-without-logrotate /var/log/dnx dnx-server.x86_64: W: log-files-without-logrotate /var/log/dnx Need to make a logrotate file and add a Requires: logrotate? dnx.x86_64: W: no-reload-entry /etc/rc.d/init.d/dnx Would be nice to have if it makes sense. Not sure if this can be reloaded though. Can it? dnx.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/dnx-0.18/README dnx-server.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/dnx-server-0.18/README Would be nice to fix with a sed. Check the wiki for recipe. dnx-doc.x86_64: W: file-not-utf8 /usr/share/doc/dnx-doc-0.18/dnx-doxy-0.18.tar.gz This could be unpacked and shipped unpacked? 3. Some of the directories here are owned in the base package and the subpackages as well. Could you describe why or rethink this? What are the use cases? doc - used by itself, shouldn't need any others? server - could be used by itself? or does it need the base package? base - by itself or not? This ties in with the users setup... the nagios package (currently required by this server subpackage) already makes a nagios user, why do that here? I also see things like the base package having files owned by nagios, but it has no dependency there or user creation. It's best to try and make only one package own dirs/files and use dependencies for subpackages that need those directories or files. Ie, if the server needs the base package we can remove some of the dual listed dirs in the server subpackage. Does that make sense? -- 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