[Bug 464016] Review Request: eclipse-findbugs - Eclipse plugin for FindBugs

[bugzilla at redhat.com]

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





--- Comment #6 from Andrew Overholt <overholt at redhat.com>  2009-03-06 16:39:37 EDT ---
Thanks for the submission.  Here's the review.  Lines beginning with X need
attention; those beginning with * are okay.  Other than a few minor things, I
think we're good to go once the dependencies are in.

* verify the final provides and requires of the binary RPMs
X make sure lines are <= 80 characters
  - please add a line continuation on line 34 to fix this
* package successfully compiles and builds
* BuildRequires are proper
 - the BR on rcp is unnecessary if pde is already there
 - I recommend dropping the gcj bits 'cause the underlying Eclipse RPMs don't
have them and they won't make much of a difference for just this plugin
* macros fine
* package is named appropriately
* it is legal for Fedora to distribute this
* license field matches the actual license.
* license is open source-compatible.
* specfile name matches %{name}
* md5sum matches upstream
* skim the summary and description for typos, etc.
* summary and description good
* correct buildroot
* %{?dist} used correctly
X license text included in package and marked with %doc
  - since upstream doesn't do this, it's not necessary to force it, but maybe
you could ask upstream to do so in the future?
X packages meets FHS (http://www.pathname.com/fhs/)
 - this should probably be in %{_datadir}/eclipse/dropins not
%{_libdir}/eclipse/dropins
X rpmlint on <this package>.srpm gives no output
 - this seems odd:

  findbugs.src:128: E: hardcoded-library-path in ../../lib/findbugs-tools.jar

* changelog format okay
* Summary tag does not end in a period
* no PreReq
* specfile is legible
* specfile written in American English
* no -doc sub-package necessary
* not native, so no rpath, static linking, etc.
* no config files
* not a GUI app
* no -devel necessary
* install section begins with rm -rf $RPM_BUILD_ROOT or %{buildroot}
* no translations so no locale handling
* Requires(pre,post) fine but recommend removing gcj bits so will not be
necessary
* package not relocatable
* package contains code
* package owns all directories and files
* no %files duplicates
* file permissions fine
* %clean present
* %doc files do not affect runtime
* not a web app
X run rpmlint on the binary RPMs => no output

 - there are a lot of warnings about non-relative symlinks.  You could fix this
by making the symlinks to the stuff in /usr/share/java ../../../ (or whatever)
instead
 - there's one warning about a . file:

eclipse-findbugs.x86_64: W: hidden-file-or-dir
/usr/lib64/eclipse/dropins/findbugs/plugins/edu.umd.cs.findbugs.plugin.eclipse_1.3.7.20081230/.options

 - is that file necessary?

* I verified that it installs and that the findbugs feature is available (after
restarting with -clean ... sigh).  I look forward to using it!

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