[Bug 603295] New: Review Request: guava - Guava (Google Common Libraries)

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

Summary: Review Request: guava - Guava (Google Common Libraries)

https://bugzilla.redhat.com/show_bug.cgi?id=603295

Summary: Review Request: guava - Guava (Google Common
                    Libraries)
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: medium
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail], [e-mail]
   Estimated Hours: 0.0
    Classification: Fedora
    Target Release: ---

Spec URL: http://huwang.fedorapeople.org/packages/guava/guava.spec
SRPM URL: http://huwang.fedorapeople.org/packages/guava/guava-05-1.src.rpm
Description: Guava is a suite of core and expanded libraries that include
utility classes, google's collections, io classes, and much much more.

Scratch built in koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2246200

[Bug 603295] Review Request: guava - Guava (Google Common Libraries)

[Red Hat Bugzilla]

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

Stanislav Ochotnicky  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

--- Comment #1 from Stanislav Ochotnicky  2010-06-14 03:08:54 EDT ---
I will do the review

[Bug 603295] Review Request: guava - Guava (Google Common Libraries)

[Red Hat Bugzilla]

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

--- Comment #2 from Stanislav Ochotnicky  2010-06-14 04:32:43 EDT ---
I planned to start official review immediately, but due to problems with this
package noted below I will delay it until those issues are fixed.

NEEDSWORK: rpmlint must be run on every package. The output should be posted in
the review.
guava.src: W: name-repeated-in-summary C Guava
guava.src: W: spelling-error %description -l en_US google's -> Google's,
goggle's, go ogle's
guava.src: W: spelling-error %description -l en_US io -> oi, Io, ii
guava.src: W: spelling-error %description -l en_US javax -> java, java x, Java
guava.src: W: invalid-url Source0: guava-r05.tar.bz2
guava.noarch: W: name-repeated-in-summary C Guava
guava.noarch: W: spelling-error %description -l en_US google's -> Google's,
goggle's, go ogle's
guava.noarch: W: spelling-error %description -l en_US io -> oi, Io, ii
guava.noarch: W: spelling-error %description -l en_US javax -> java, java x,
Java
guava.noarch: W: non-conffile-in-etc /etc/maven/fragments/guava
guava.noarch: W: dangling-relative-symlink /usr/share/java/guava-r05.jar *-05*
3 packages and 0 specfiles checked; 0 errors, 11 warnings.

So let's go 1 by 1:
 - summary should normally not repeat name of package. So simplify it
 to something like "Google Core Libraries for Java"
 - "Google's" not "google's"
 - your symlink in /usr/share/java is completely wrong. You will have
 to play with it a bit.

BIG FAT WARNING:
I have noted this before in previous reviews (not sure if it was your
package). When you do:
> svn export http://guava-libraries.googlecode.com/svn/trunk/ guava-r05

You CAN NOT guarantee checked out sources will always be the same! In
fact checked out sources are very different from sources you'd get by doing:
> svn export http://guava-libraries.googlecode.com/svn/tags/release05 guava-r05

And that you should do. Trunk is always changing, so you cannot make releases
out of it without specifying at least revision number (if no tags exist).

NEEDSWORK: The package must meet the Packaging Guidelines .

Don't use:
> ln -s /usr/share/java/jsr-305.jar lib/jsr305.jar

Instead use something like
export CLASSPATH=$(build-classpath jsr-305)

or if the build system really needs that jar file in lib/, see:

https://fedoraproject.org/wiki/Packaging:Java#build-jar-repository

I'll do full official review after these problems have been fixed.

[Bug 603295] Review Request: guava - Guava (Google Common Libraries)

[Red Hat Bugzilla]

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

--- Comment #3 from Stanislav Ochotnicky  2010-06-15 05:24:18 EDT ---
I just realized one more thing you will have to do. Provide depmap for groupId:
com.google.collections artifactId: google-collections. This is so that mvn-jpp
runs will be able to work without modifications.

[Bug 603295] Review Request: guava - Guava (Google Common Libraries)

[Red Hat Bugzilla]

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

--- Comment #4 from huwang  2010-06-17 07:57:48 EDT ---
All are fixed. Thank you for your suggestions. Please review, thanks.
Spec URL: http://huwang.fedorapeople.org/packages/guava/guava.spec
SRPM URL: http://huwang.fedorapeople.org/packages/guava/guava-05-2.src.rpm
Scratch built in koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2255231

[Bug 603295] Review Request: guava - Guava (Google Common Libraries)

[Red Hat Bugzilla]

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

--- Comment #5 from Stanislav Ochotnicky  2010-06-18 05:03:08 EDT ---
Unfortunately that symlink  in /usr/share/java is still wrong. This whole part:
install -Dpm 644 build/dist/guava-r%{version}/%{name}-r%{version}.jar  
%{buildroot}%{_javadir}/%{name}-r%{version}.jar

(cd %{buildroot}%{_javadir} && for jar in *-%{version}*; \
    do ln -sf ${jar} `echo $jar| sed "s|-%{version}||g"`; done)

Needs to be modified to suit name of the jar file. The for cycle and sed call
are not used properly because there is "r" in the jar name before version.
Other than that package looks OK. Fix this one issue and I can approve it.

[Bug 603295] Review Request: guava - Guava (Google Common Libraries)

[Red Hat Bugzilla]

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

--- Comment #6 from huwang  2010-06-18 09:34:26 EDT ---
(In reply to comment #5)
> Unfortunately that symlink  in /usr/share/java is still wrong. This whole part:
> install -Dpm 644 build/dist/guava-r%{version}/%{name}-r%{version}.jar  
> %{buildroot}%{_javadir}/%{name}-r%{version}.jar
>
> (cd %{buildroot}%{_javadir} && for jar in *-%{version}*; \
>     do ln -sf ${jar} `echo $jar| sed "s|-%{version}||g"`; done)
>
> Needs to be modified to suit name of the jar file. The for cycle and sed call
> are not used properly because there is "r" in the jar name before version.
> Other than that package looks OK. Fix this one issue and I can approve it.

Fixed. Please review again, thanks.
Spec URL: http://huwang.fedorapeople.org/packages/guava/guava.spec
SRPM URL: http://huwang.fedorapeople.org/packages/guava/guava-05-3.src.rpm
Scratch built in koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2257019

[Bug 603295] Review Request: guava - Guava (Google Common Libraries)

[Red Hat Bugzilla]

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

Stanislav Ochotnicky  changed:

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

--- Comment #7 from Stanislav Ochotnicky  2010-06-18 09:53:51 EDT ---
Package looks good now.

APPROVED

[Bug 603295] Review Request: guava - Guava (Google Common Libraries)

[Red Hat Bugzilla]

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

--- Comment #8 from huwang  2010-06-19 21:24:13 EDT ---
Thanks for the review.

New Package CVS Request
=======================
Package Name: guava
Short Description: Google Core Libraries for Java
Owners: huwang
Branches: 
InitialCC:

[Bug 603295] Review Request: guava - Guava (Google Common Libraries)

[Red Hat Bugzilla]

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

huwang  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

[Bug 603295] Review Request: guava - Guava (Google Common Libraries)

[Red Hat Bugzilla]

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

--- Comment #9 from Kevin Fenzi  2010-06-20 22:20:35 EDT ---
CVS done (by process-cvs-requests.py).

[Bug 603295] Review Request: guava - Guava (Google Common Libraries)

[Red Hat Bugzilla]

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

huwang  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE

--- Comment #10 from huwang  2010-06-20 23:44:17 EDT ---
Built in koji : http://koji.fedoraproject.org/koji/buildinfo?buildID=178892