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 #13 from David A. Wheeler <dwheeler(a)dwheeler.com> 2010-05-24 12:08:08
EDT ---
I'm looking over the .spec file for overall comments, before I delve into a
point-by-point comparison with the guidelines.
* Every time you make a change you should bump the "Revision" number,
and post the new URL. That'll be *critical* when it's submitted to the
repository, but it'd reduce confusion now too.
* Patch0 comment says:
"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 confused, there *is* an ocaml-ocamlgraph package.
Is the Fedora respository's ocamlgraph the wrong (too old) version?
Or, is this just an old comment based on an older version of this spec,
now OBE?
* Patch1 isn't prefixed with "frama-c-", and that can cause trouble.
In general, if someone looks in their rpmbuild/SOURCES, they can get
confused (and namespace issues could result) if they aren't prefixed.
You should prefix the patch filenames with NAME- (e.g., "frama-c-").
* Rejected Debian Patch2 (0002-Make-Jessie-plugin-use-Jc-from-Why-2.19.patch)
comment says, "I don't know why we would want to use that
Jessie -> not including this patch". You're not including that patch
because we want to use a later version of Why, correct? If so, just say:
"We use a later version of Why, so this patch is irrelevant".
Although I applaud "I don't know..." comments for their honesty, I think
we should track them down so you can simply say why something was done.
In this case, there's a good reason: The patch is for an obsolete
version of "why".
* Rejected Debian patch "0003-Add-dGraphView.cmo-when-linking.patch"
says "I'm not sure quite what this patch does, but I'm going to
leave it out for now". Need to track this down a little of what this is.
My *guess* from the text is that it merges graphview into another
another library, and we *don't* want to do that (because then upgrading
graphview won't updated the merged file). If that's the
situation, please say so directly.
* Rejected Debian patch 0004-Use-GSourceView2.patch comment says
"I didn't see any Fedora package providing the files that this patch
references -> dropping for now".
Did you check out "gtksourceview2-devel" and "gtksourceview2"?
It's possible that the patch includes the *OCaml* bindings for these
and that Fedora doesn't include those bindings. If so, just say that.
* In Patch "frama-c-1.4-ptests-fix-for-ocaml-3.11.2.patch" there
needs to be a comment explaining (1) what it's for, and (2)
when you submitted it upstream (and if not, why not). Shouldn't be *long*,
but it should say *something*.
* Nit: Your "Requires:" statements have inconsistent indenting.
Please fix while you're at it.
* Nit: "plug in's" is not a word. An "'s" is either a
possessive or
an appreviation of "is", and this is neither (it's just a plural).
I'd suggest "plug-ins" instead. If the spellchecker whines, too bad.
* I notice that the entire "%check" section is commented out.
A %check section is NOT required, and I'd prefer the comments to nothing,
but it'd be nice if it worked :-).
* As noted earlier, the .byte files don't work, and probably shouldn't
be packaged at all.
* I'll check the %files list separately.
* In %post, just remove the comment:
#chcon -t textrel_shlib_t '%{_libdir}/frama-c/plugins/Ltl_to_acsl.cmxs'
I don't see that the commend adds anything.
* The %changelog doesn't list "Mark Rader"; it should!
--
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.