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=564520 --- Comment #4 from David A. Wheeler <dwheeler@xxxxxxxxxxxx> 2010-02-15 13:28:06 EST --- Here are some initial comments from reading the .spec file: * Thanks for working out the complicated licensing situation. Hopefully Fedora legal will reply soon re: QPL with modifications. * I suggest emailing Medhi Dogguy re: the Debian patches, and then changing the comments to document with certainty the decisions. E.G.: # This seems to apply for us as well, keeping Debian filename should be (once you've confirmed): # This applies to us as well... # I don't know why we would want to use that Jessie -> not including this patch should be: # We won't embed Jessie, but will instead use the Jessie supplied by the Why package # I'm not sure quite what this patch does, but I'm going to leave it out for now :-). # I didn't see any Fedora package providing the files that this patch # references -> dropping for now I don't have the patch you dropped, but it's worth noting that Fedora 12 provides packages named gtksourceview *and* gtksourceview2, as well as their -devel packages. Are these what you're looking for? In general, I know that people prefer relatively few lines in .spec files. * You need to rename this source (patch) file: 0001-Fix-weak-pattern-matching-in-dynlink_lower_311_byte..patch First, the ".." should be ".", and the prefix should be %{name}-%{version}- so that multiple simultaneous builds in the same SOURCE directory won't cause trouble. * You should have a brief "why you need this" comment above: %if 0%{?fedora} >= 13 Patch2: frama-c-1.4-ptests-fix-for-ocaml-3.11.2.patch %endif * As I noted earlier, you need gtksourceview-devel as a BuildRequires, or the build may fail. * You use a macro here inside a comment, which will trigger a warning: # %check # make ptests %{frama-c_make_options} # make oracles %{frama-c_make_options} I suggest using "with" to make these disabled by default, but enable-able. E.G.: %if %{with lengthy-tests} %check make ptests %{frama-c_make_options} make oracles %{frama-c_make_options} %endif It'd be even better if you had at least a few tests run even without enabling "lengthy-tests", but this would at least make it easy to enable the lengthy test and would eliminate rpmlint complaints. * The package includes over 20M of documentation; I think the documentation should be in a separate subpackage. * It starts with "# Some ideas for this spec file taken from a prior attempt by Richard W. M. Jones" I think it's great to give credit, but you might consider moving that to the bottom as part of the comments in the ChangeLog. Not a requirement. -- 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