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=592579 --- Comment #35 from David A. Wheeler <dwheeler@xxxxxxxxxxxx> 2010-07-16 17:49:31 EDT --- Okay, I've started reviewing. This new package builds on my system, and the only rpmlint messages were the ones noted above. Comment 19 noted a number of "to-do" items, so I'm going to first go through all of the ones that had problems before: * "I don't understand why this package installs "/usr/bin/ptests.byte"." That's now gone, good. * "Excluding the ".byte" files is fine. But that means that this package won't work on some architectures, so you need some ExcludeArch statements." It now has an ExcludeArch statement, and I notice that you did a Koji test (that should tell you of problems like this). * "I *still* don't understand this comment..." "The configure file uses graph.cmo in compiling a test program to check for ocamlgraph, but Fedora doesn't include this file in its distributions." I'm sorry, the comment changed, but new comment somehow got messed up and I *still* don't understand it. It now says: # The configure file uses graph.cmo in compiling a test program to # check for ocamlgraph, but Fedora doesn't include this file in itshas changed the # extension in later distributions to graph.cma . But "rpmls ocaml-ocamlgraph" shows: -rw-r--r-- /usr/lib/ocaml/ocamlgraph/graph.cma -rw-r--r-- /usr/lib/ocaml/ocamlgraph/graph.cmi -rw-r--r-- /usr/lib/ocaml/ocamlgraph/graph.cmo The Fedora package *does* include the ".cmo" *and ".cma" file. I still don't understand what this spec is trying to say. (I realize that you inherited this package from others, so this is probably inherited as well, but it sure isn't clear as it is.) * "I really think you should prefix the filename of Patch1 with "frama-c-"." Fixed, thanks!! * "Patch2 (ptests) has *no* explanation, and that is NOT okay." All patches now have an explanation, good! * "Nit: "plug in's" is still not a word." Using "plug ins" is not my preference, but I understand that you're trying to keep the spellchecker happy, so fine. * "No %check section. It's not required..." Still no %check section, but since it's not required... it's not required :-). * "I'll check the %files list separately." Which I still need to do. * "You really need to do a Koji build (see comment #18)...." Which you did, see comment 34. Thanks. * "MUST: rpmlint must be run on every package. The output should be posted in the review.[1]" OK. * MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines . OK, see comment 32. I don't know why legal hasn't responded yet, they really should. But they approved the QPL, and the only changes cannot affect its Freeness (see comment 32). I'd like to hear anyone else's comments about this. * MUST: The License field in the package spec file must match the actual license. [3] OK. * MUST: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc.[4] OK. I checked the binary package with rpmls, and they're there. * MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture. [7] OK. I just did it, "works for me". * MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch. Each architecture listed in ExcludeArch MUST have a bug filed in bugzilla, describing the reason that the package does not compile/build/work on that architecture. The bug number MUST be placed in a comment, next to the corresponding ExcludeArch line. [8] OK. Added ExcludeArch, tested with Koji. * MUST: All build dependencies must be listed in BuildRequires, except for any that are listed in the exceptions section of the Packaging Guidelines ; inclusion of those as BuildRequires is optional. Apply common sense. OK. Worked with Koji. * MUST: Packages must NOT bundle copies of system libraries.[11] OK. As noted in earlier comments, this has a forked version of cil; I don't see any way around this :-(, and you have a reasonable justification in the spec comments. * MUST: A Fedora package must not list a file more than once in the spec file's %files listings. [14] OK. I didn't find any dups this time. * MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present. [18] OK. We now have some files marked as documentation. * MUST: Header files must be in a -devel package. [19] OK. * MUST: Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. If you feel that your packaged GUI application does not need a .desktop file, you must put a comment in the spec file with your explanation. [22] OK, we now have a .desktop file. This is looking really good. I want to test it out, and do a clean read-through. But other than my complaint about a comment (which I *still* don't understand) it's looking great so far. -- 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