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=785727
Hans de Goede <hdegoede(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flag| |fedora-review+
--- Comment #6 from Hans de Goede <hdegoede(a)redhat.com> 2012-03-09 03:20:01 EST ---
Good:
====
- rpmlint checks return:
ocaml-camlimages-4.0.1-1.fc17.src.rpm
ocaml-camlimages.src:89: W: macro-in-comment %exclude
ocaml-camlimages.src:89: W: macro-in-comment %{_libdir}
ocaml-camlimages.src:103: W: macro-in-comment %{_libdir}
ocaml-camlimages.src: W: invalid-url Source1: camlimages-2.2.0-htmlref.tar.gz
3 packages and 0 specfiles checked; 0 errors, 4 warnings.
These can all be ignored.
- package meets naming guidelines
- package meets packaging guidelines
- license (GPLv2 with exceptions) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- devel package ok
- no .la files
- post/postun ldconfig not needed since no libs directly under %{_libdir}
- devel requires base package n-v-r
Should Fix
======
-Drop these 3 lines (use rpms internal dep generator instead):
%define _use_internal_dependency_generator 0
%define __find_requires /usr/lib/rpm/ocaml-find-requires.sh
%define __find_provides /usr/lib/rpm/ocaml-find-provides.sh
-Make opt unconditional (not 100% sure about this, but if lablgtk does it can
probably be dropped and
it makes the spec easier to read)
-Using:
https://bitbucket.org/camlspotter/camlimages/get/v4.0.1.tar.gz as
Souce0 works fine for me,
wget gets a v4.0.1.tar.gz, spectool -g gets a v4.0.1.tar.gz and rpmlint likes
it (iow it can download and
verify the source)
-Doing rm -rf buildroot in %install and having a %clean is no longer needed now
a days, and should not
be done when not manually defining buildroot inside the spec
- -devel packages should require the main package like this:
Requires: %{name}%{?_isa} = %{version}-%{release}
So please add %{?_isa} there.
-You could also drop the 2 %defattr lines, those are also not needed now a days
Since there are no blockers, this package is APPROVED! But please do consider
fixing some of the should fix items.
--
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.