[Bug 464014] Review Request: findbugs - Find bugs in Java code

Thu Mar 5 20:48:48 UTC 2009

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=464014

Lillian Angel <langel at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |needinfo?(loganjerry at gmail.
                   |                            |com)

--- Comment #10 from Lillian Angel <langel at redhat.com>  2009-03-05 15:48:47 EDT ---
(In reply to comment #8)
> Lillian, thanks for the review.  I've been wanting to get this package into
> Fedora for a very long time.  It's good to see that we're almost done with its
> dependencies and have movement on the package itself.  Here are my responses to
> the items raised above:
> 
> >   XXXX 1.4 No inclusion of pre-built binaries or libraries
> >         The lib/*.jars and questionable files should be removed from the zip
> > prior to uploading it. Please recreate the zip.
> 
> I do not know of any questionable files in the source release.  To what do you
> refer?
> 
> By my reading of https://fedoraproject.org/wiki/Packaging/SourceURL, I have to
> use the unmodified zip file unless that file contains prohibited elements.  The
> jars in the lib directory are all released under licenses that allow
> redistribution.  This includes AppleJavaExtension.jar, whose license
> information (included in lib/LICENSE_AppleJavaExtensions.txt) shows that that
> jar is distributed under a slightly modified MIT license.  Unless I have
> misunderstood something, I must not modify the zip file.  If I have
> misunderstood, please help me understand.
> 
> Note that the spec file deletes all the jar files in the %prep stage, so none
> of them are used in building.

As long as all the files are redistributable, this is ok. Thanks for the
explanation.

> 
> >   XXXX 1.15 Requires
> >  Have each on a separate "Requires" line. 
> >   XXXX 1.16 BuildRequires
> >  Have each on a separate "BuildRequires" line.
> 
> I do prefer keeping them on one line.  In my typical monitor setup, vertical
> space is more precious than horizontal space.  Nevertheless, this is such a
> minor point that I don't really want to argue about it, so I'll do it your way.

As I said, I wont reject the review based on this. If you would prefer to group
them in such a way, that is fine. Comments can help to explain the groupings.
We can leave this as is.

> 
> >   XXXX 1.17 Summary and description
> >         Can you shorten the tools description. This is too much information-
> > possibly remove the class names etc.
> 
> This information appears nowhere else.  If I take it out of the description,
> where do you suggest I put it?

Don't take out the description, please modify it. It should be short and
concise.

> 
> >   XXXX 1.23 Requiring Base Package
> >  ok, but please put all "Requires" on a separate line
> 
> Done.
> 
> Now for the rpmlint complaints:
> 
> > ant-findbugs.noarch: W: non-standard-group Development/Build Tools
> 
> This is the group of ant itself, also maven.  Since groups aren't consumed by
> any Fedora tools (they use comps.xml instead), the group really doesn't matter
> anyway.  I think this choice is appropriate so that it corresponds to ant.
> 

ok, that's fine.

> > findbugs.noarch: W: wrong-file-end-of-line-encoding /usr/share/doc/findbugs-1.3.7/doc/manual_ja.xml
> 
> I included the entire doc directory in %doc.  But this file is input to
> docbook, and shouldn't appear in the binary rpm.  It looks like the files named
> manual* are no longer needed once the manual is built.  I'll delete them.

Or run dos2unix on the file

> 
> > findbugs.noarch: W: class-path-in-manifest /usr/share/java/findbugs-1.3.7.jar
> 
> The ClassPath entry in the manifest is necessary to find dependent jars.  The
> rpmlint complaint about older Java versions no longer applies to any supported
> Fedora release, nor even to the latest RedHat EL.  It is true that versioned
> ClassPath entries are inflexible, but note that I used only unversioned
> entries.

see Andrew's comment above.

> 
> > findbugs.src:109: E: hardcoded-library-path in ../../lib/findbugs-tools.jar
> 
> This is bogus.  That is the lib directory in the source distribution, not /lib
> or /usr/lib.

ok.

> 
> > findbugs.src: W: non-coherent-filename findbugs-1.3.7-2.src.rpm findbugs-1.3.7-2.fc10.src.rpm
> 
> That's just a filename problem on my web site, not a spec file problem.  It
> will have no effect on koji builds.  I'll give you a well-named file below.

thanks

> 
> > findbugs-javadoc.noarch: W: non-standard-group Development/Documentation
> 
> Same remark as the one above about the main package group.

ok

> 
> New versions are here:
> Spec URL: http://jjames.fedorapeople.org/findbugs/findbugs.spec
> SRPM URL: http://jjames.fedorapeople.org/findbugs/findbugs-1.3.7-3.fc10.src.rpm

-- 
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.