[Bug 444512] Review Request: eclipse-eclemma - EMMA plugin for Eclipse

[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=444512





--- Comment #4 from Andrew Overholt <overholt at redhat.com>  2008-12-03 11:25:01 EDT ---
(In reply to comment #3)
> The review arrived finally. Nothing serious, just few small things:

Thanks!

> * First you should close bug #444511

Done.

> * This package is only for F-10+, right?

Yes.  I updated the BRs/Rs on eclipse stuff to make it F-10+.

> * To simplify the code, you can use
>     %define install_loc  %{_datadir}/eclipse/dropins/eclemma
> and update everything accordingly. This is a suggestion, not a requirement.

Thanks, done.

> * You are now not owning the directory
>     %{install_loc}/eclemma
> With the above suggestion you can just use
>     %{install_loc}
> in the files section.

Yup, done.

> * rpmlint says:
>     eclipse-eclemma.noarch: W: no-documentation
>        Please add those about.html files (rename them), and at least the
> license.html and faq.html files to %doc

Done.

>     eclipse-eclemma.noarch: W: dangling-symlink
> /usr/share/eclipse/dropins/eclemma/eclipse/plugins/com.mountainminds.eclemma.core_1.3.2/emma.jar
> /usr/share/java/emma.jar
>         This can be ignored.
>     eclipse-eclemma.noarch: W: symlink-should-be-relative
> /usr/share/eclipse/dropins/eclemma/eclipse/plugins/com.mountainminds.eclemma.core_1.3.2/emma.jar
> /usr/share/java/emma.jar
>         This should be fixed.

I think we'd be better off fixing build-jar-repository.  If you want, I'll make
it a big long ln -s ../../../java/emma.jar instead, but build-jar-repository
seems cleaner.

>     eclipse-eclemma.noarch: W: hidden-file-or-dir
> /usr/share/eclipse/dropins/eclemma/eclipse/plugins/com.mountainminds.eclemma.core_1.3.2/.options
>         Is this file required?

It may be just a build-time requirement but it doesn't harm anything to keep
it.

>     eclipse-eclemma.src: W: strange-permission get-eclemma.sh 0775
>         Please use 644 for source files.

This is a shell script to fetch the source.  Do you still want it changed?

> * The file 
>     ./com.mountainminds.eclemma.core/emma.jar
> needs to be removed in the %prep

Since I was symlinking over it I didn't think it was a big deal but I've added
an explicit removal before the ln line now.

> * The license file says:
>     "The user documentation contains example code taken from the Apache Jakarta
> Commons project, provided under the terms and conditions of the Apache License
> Version 2.0. "
>        Shall we include ASL 2.0 in the license tag?

I don't know.  I guess it can't hurt :)  I've added it.

> * Each package must consistently use macros, as described in the macros section
> of Packaging Guidelines .
> You are using $RPM_BUILD_ROOT at certain points and %{buildroot} on others. You
> should stay consistent.

Fixed.

Updated spec and SRPM:

http://fedorapeople.org/~overholt/eclipse-eclemma.spec
http://fedorapeople.org/~overholt/eclipse-eclemma-1.3.2-2.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.