https://bugzilla.redhat.com/show_bug.cgi?id=1055398 --- Comment #6 from Michel Alexandre Salim <michel@xxxxxxxxxxxxxxx> --- (In reply to Jerry James from comment #5) > Review done on Fedora 20, since the Rawhide buildroot is currently broken > due to an incomplete icu update. > > Issues, in no particular order: > 1) Missing BRs on ocaml-biniou-devel and ocaml-easy-format-devel. Indeed, I must have forgotten to add them, since by submitting review requests for them I obviously know they are required :p Brain fade... > 2) BR on ocaml-findlib-devel should just be on ocaml-findlib. > 3) Add "ExclusiveArch: %{ocaml_arches}". Will this be a problem, since > zeroinstall-injector was available for all arches? That is, non-ocaml > arches will not have an upgrade path available to them. > 4) Remove the "rm -rf $RPM_BUILD_ROOT" at the top of %install. > 5) Don't strip the cmxs files. Same as the ocaml- packages > 6) The latest changelog entry says "Update to 2.6.1", but it appears that > the > package is actually for 2.6, not 2.6.1. Ouch! > 7) Speaking of the changelog, I question the usefulness of keeping the > changelogs from the previous package. Think about whether it would be > better to start over on the changelog. Good idea > 8) The spec file gives the license as "LGPLv2", but README.md contains the > "any later version" clause. Shouldn't the license be "LGPLv2+"? Good catch. Will peruse the source code files before posting the next revision > 9) The spec has a BR on ocaml-dbus-devel, but I see this in the build log: > > obus not found; compiling without D-BUS support > > I think it wants this: http://forge.ocamlcore.org/projects/obus/. Does > that mean that the BR on ocaml-dbus-devel is unneeded? Indeed, I see no > references in the code to dBus, which is what ocaml-dbus-devel provides. Ah, something else to package then. Good thing it's optional since ocaml-dbus is the only dbus-related package we carry right now. Will work on that after this, and enable the dependency later on > 10) "Provides" for the old package name are great, but I don't think that > "Obsoletes" is correct. Doing it this way means that each time you bump > NEVR on the new package, it will obsolete the previous version of itself. > Yep, forgot to hard-code the last available version there > 11) In %description, use the American spelling of centralized, decentralized, > and virtualization. Oh good catch, thanks. The author is British, and having spent some years there myself my radar for British-ism is really off. > 12) Regarding these rpmlint messages: > > 0install.src:88: E: hardcoded-library-path in > %{_prefix}/lib/0install.net/*.cmxs > 0install.src:124: E: hardcoded-library-path in %{_prefix}/lib/0install.net > > Those are architecture-specific files. Shouldn't they be in %{_libdir}? > Indeed. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review