[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 #7 from Jerry James <loganjerry at gmail.com>  2009-03-06 18:42:02 EDT ---
Thanks for the review, Andrew.  I'm excited about getting this into Fedora.

> X make sure lines are <= 80 characters
>   - please add a line continuation on line 34 to fix this

When I do that, I get an embedded newline in the definition of %plugin_dir,
which causes the symbolic link commands to break.  If you know of some way to
do this without picking up the embedded newline, please let me know.

> * BuildRequires are proper
>  - the BR on rcp is unnecessary if pde is already there

Ah, missed that.  Thanks.

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

Okay, they're gone.

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

Sure, I'll ask.  This upstream is funny.  My typical interaction with them is
to ask a question, which goes unanswered for months.  Then I'll remember that I
never got an answer, follow up to it on their mailing list asking for
responses, and THEN I'll get a response.

> X packages meets FHS (http://www.pathname.com/fhs/)
>  - this should probably be in %{_datadir}/eclipse/dropins not
> %{_libdir}/eclipse/dropins

I didn't even pick up on the existence of %{_datadir}/eclipse/dropins from the
Eclipse guidelines.  I'll have to go look at them again.  Fixed.

> X rpmlint on <this package>.srpm gives no output
>  - this seems odd:
> 
>   findbugs.src:128: E: hardcoded-library-path in ../../lib/findbugs-tools.jar

??  That's from the findbugs SRPM, not the eclipse-findbugs SRPM, right?

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

Fixed.

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

Only at build time.  I don't know why it is getting packaged up.  I have now
removed it from the final binary RPM.

New versions are here:
http://jjames.fedorapeople.org/eclipse-findbugs/eclipse-findbugs.spec
http://jjames.fedorapeople.org/eclipse-findbugs/eclipse-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.