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=803089 Michael Scherer <misc@xxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |misc@xxxxxxxx Flag| |fedora-review? --- Comment #1 from Michael Scherer <misc@xxxxxxxx> 2012-03-17 18:05:16 EDT --- Let me start the review, but I am not a ocaml specialist. 1) %{_libdir}/whenjobs/ is unowned, you should add %dir %{_libdir}/whenjobs/ in the %files section 2) given that this requires f17 to be built, I think you can remove %defattr ( unless the plan is to backport to EL 5, but I doubt ) 3) same goes for %clean and the rm -Rf $RPMBUILDROOT at the start of %install, if I am not wrong, so you can remove them ( no need to keep unused cruft ) 4) per https://fedoraproject.org/wiki/Packaging/Guidelines#File_Dependencies , could you change the requires on /usr/bin/ocamlc to the package name ? same goes for the buildRequires on perl-doc. 5) nitpicking, but BuildRequires: pcre-devel, ocaml-pcre-devel is better on 2 lines, as this ease review of a potential changes. But that's not blocking for the review. 6) shouldn't whenjobsd be start at boot, with a systemd file ? ( given your blog posts and the use case, this sound logical to me, but maybe I missed something ) 7) you detect if the bytecode is created or not at the beggining of the spec, but do not act on it as explained on : https://fedoraproject.org/wiki/Packaging:OCaml#Bytecode-only_architectures So either there is something missing, or something not used, or something magic I would bet on the 1st, but maybe that's the 3rd -- 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