[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


Andrew Overholt <overholt at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+




--- Comment #8 from Andrew Overholt <overholt at redhat.com>  2009-03-09 09:35:02 EDT ---
(In reply to comment #7)
> 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.

Bah, just leave it > 80 :)

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

Thanks.  This isn't a blocker.

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

Great, thanks.

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

:)  Sorry.  Your new SRPM gives no rpmlint output.

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

I still get "dangling-relative-symlink" but I think these are all correctly
relative now.

Okay, this package is ready to go.  Once the dependencies are in, this can go
in.  I'll mark it accepted but obviously it won't build until findbugs is in. 
Thanks, Jerry!

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