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=460244 --- Comment #1 from Alan Dunn <amdunn@xxxxxxxxx> 2008-08-26 20:15:14 EDT --- Bugzilla was inoperative at the time that I originally produced this package, so I solicited a review by email (from David Wheeler, who will vouch for this). He wrote: * I could NOT rebuild on i386; it seems to fail on ocaml-ocamlgraph. Perhaps the ocaml-ocamlgraph I'm using is not the latest version, but in any case, I got this error: "Cannot find file graph.cmxa". I rebuilt http://www.duke.edu/~amd34/ocamlgraph/ocaml-ocamlgraph-0.99c-2.fc9.src.rpm to create ocaml-ocamlgraph, and it's true, there's no .cmxa file: $ rpm -qil ocaml-ocamlgraph ... /usr/lib/ocaml/ocamlgraph /usr/lib/ocaml/ocamlgraph/META /usr/lib/ocaml/ocamlgraph/graph.cma /usr/lib/ocaml/ocamlgraph/graph.cmi /usr/share/doc/ocaml-ocamlgraph-0.99c/LICENSE ... which I fixed in version 2 by adding ocaml-ocamlgraph-devel to BuildRequires (it is not yet in the repositories, which implies that I can't really use mock) * In "cp %{SOURCE1} %{SOURCE2} .", say "cp -p" instead. You should try to keep the timestamps where you can. Indeed, if you got these files from elsewhere, try to preserve THEIR timestamps. which I also changed, and the following two comments * The spec file says this, and I couldn't figure out what "->" meant; can you make it clearer by replacing "->" with the word(s) you mean? # Avoid a Makefile patch by just adding this empty file -> autoconf # doesn't complain (better than ignoring all status from configure) * I think the "iconv" should be run during _build_, not _install_. Also, you have 3 semicolon-separated commands on one line to invoke it, which kindof hides the "iconv". I'd prefer to have that as a sequence of 3 commands on 3 lines. I'm not big on cuddling multiple commands on a line anyway, but this sequence hides the important command: "mv CeCILL-C CeCILL-C.iso8859; iconv -f ISO-8859-1 -t UTF-8 < CeCILL-C.iso8859 > CeCILL-C; rm CeCILL-C.iso8859" which I also changed. He then gave a "more formal review": + rpmlint output It outputs: alt-ergo.i386: W: invalid-license CeCILL-C But this is an error in rpmlint (CeCILL-C is a recent addition), already explained in the spec file + package name satisfies the packaging naming guidelines + specfile name matches the package base name + package should satisfy packaging guidelines + license meets guidelines and is acceptable to Fedora Yes. CeCILL-C was just added to the acceptable list. + license matches the actual package license + %doc includes license file Yes, /usr/share/doc/alt-ergo-0.8/CeCILL-C + spec file written in American English + spec file is legible + upstream sources match sources in the srpm Yes, sha1sum: 6a073b88d799e3dfcc7e13dfc1386c6422ce9ab8 + package successfully builds on at least one architecture i386. Can't try koji until ocamlgraph is in. ??? ExcludeArch bugs filed ??? BuildRequires list all build dependencies (Not yet) n/a %find_lang instead of %{_datadir}/locale/* n/a binary RPM with shared library files must call ldconfig in %post and %postun + does not use Prefix: /usr + package owns all directories it creates + no duplicate files in %files + %defattr line + %clean contains rm -rf $RPM_BUILD_ROOT + consistent use of macros + package must contain code or permissible content n/a large documentation files should go in -doc subpackage + files marked %doc should not affect package n/a header files should be in -devel n/a static libraries should be in -static n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig' n/a libfoo.so must go in -devel n/a -devel must require the fully versioned base n/a packages should not contain libtool .la files n/a packages containing GUI apps must include %{name}.desktop file + packages must not own files or directories owned by other packages + %install must start with rm -rf %{buildroot} etc. + filenames must be valid UTF-8 Optional: n/a if there is no license file, packager should query upstream n/a translations of description and summary for non-English languages, if available ??? reviewer should build the package in mock Don't know how to do that yet; ocamlgraph not available in yum ??? the package should build into binary RPMs on all supported architectures Don't know how to do that yet; ocamlgraph not available in yum + review should test the package functions as described Tried using gwhy; looks great! n/a scriptlets should be sane n/a pkgconfig files should go in -devel + shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or /usr/sbin -- 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