Bug 560170 - Review Request: jcodings - Java-based codings helper classes for Joni and JRuby
Summary: Review Request: jcodings - Java-based codings helper classes for Joni and JRuby
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mo Morsi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 561464 (view as bug list)
Depends On:
Blocks: 541638 560169 560172 561482 561484
TreeView+ depends on / blocked
 
Reported: 2010-01-30 02:26 UTC by Victor G. Vasilyev
Modified: 2010-02-11 19:48 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-02-11 19:48:52 UTC
Type: ---
mmorsi: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Victor G. Vasilyev 2010-01-30 02:26:20 UTC
Spec URL: http://victorv.fedorapeople.org/files/jcodings.spec
SRPM URL: http://victorv.fedorapeople.org/files/jcodings-1.0.2-1.fc13.src.rpm
Description: 
Java-based codings helper classes for Joni and JRuby.


This review request has been filed, because the package was last updated more
than three months ago and it is orphaned:
https://admin.fedoraproject.org/pkgdb/packages/name/jcodings

Comment 1 Victor G. Vasilyev 2010-02-01 20:28:58 UTC
The rpmlint tool shows 0 errors 0 warnings against both packages:
jcodings-1.0.2-1.fc13.src.rpm
jcodings-1.0.2-1.fc13.noarch.rpm

Successful koji build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1957311

Comment 2 Thomas Spura 2010-02-04 13:27:15 UTC
*** Bug 561464 has been marked as a duplicate of this bug. ***

Comment 3 Victor G. Vasilyev 2010-02-05 06:29:38 UTC
Self-review:
http://victorv.fedorapeople.org/files/jcodings.self-review.txt

Comment 4 Mo Morsi 2010-02-08 18:49:17 UTC
Looks good for most part, builds find in mock/koji and passes most review guidelines. 

A couple of specifics though:

Even though this is in the orphaned specfile, the following line doesn't work (try it yourself):
find -name '*.jar' -o -name '*.class' -exec rm -f '{}' \;

You can replace it w/ this if you want:
find \( -name '*.class' -o -name '*.jar' \) -exec rm -rf {} \;

or this:
find ./ -name '*.jar' -exec rm -f '{}' \;
find ./ -name '*.class' -exec rm -f '{}' \;

As far as the changelog you have a typo, "Jun 2010" should be "Jan 2010". 

Also why did you remove the previous entries from the changelog http://cvs.fedoraproject.org/viewvc/rpms/jcodings/F-11/jcodings.spec?view=markup

I would leave those in and add a bit about gcj support being removed for jcodings. I had tried getting gcj/jcodings working myself to no avail, aot-compile-rpm blocks indefinetly, consuming more and more resources. If you can figure out the problem, adding gcj support is easy, if not just add a note to the changelog.

http://fedoraproject.org/wiki/Packaging/GCJGuidelines

Comment 5 Victor G. Vasilyev 2010-02-09 00:01:40 UTC
(In reply to comment #4)
First of all, thanks for review.

> Looks good for most part, builds find in mock/koji and passes most review
> guidelines. 
> 
> A couple of specifics though:
> 
> Even though this is in the orphaned specfile, the following line doesn't work
> (try it yourself):
> find -name '*.jar' -o -name '*.class' -exec rm -f '{}' \;
> 
> You can replace it w/ this if you want:
> find \( -name '*.class' -o -name '*.jar' \) -exec rm -rf {} \;
> 
> or this:
> find ./ -name '*.jar' -exec rm -f '{}' \;
> find ./ -name '*.class' -exec rm -f '{}' \;
fixed as described.
> 
> As far as the changelog you have a typo, "Jun 2010" should be "Jan 2010".
fixed. 
> 
> Also why did you remove the previous entries from the changelog
> http://cvs.fedoraproject.org/viewvc/rpms/jcodings/F-11/jcodings.spec?view=markup
The clause with "find" was only a line that has not been changed in the spec. So, I've considered it as a new package, and a history, from my viewpoint, is not interesting for anybody. Nevertheless, if you prefer then I'll save the changelog.
> 
> I would leave those in and add a bit about gcj support being removed for
> jcodings. I had tried getting gcj/jcodings working myself to no avail,
> aot-compile-rpm blocks indefinetly, consuming more and more resources. If you
> can figure out the problem, adding gcj support is easy, if not just add a note
> to the changelog.
> 
> http://fedoraproject.org/wiki/Packaging/GCJGuidelines
As the package was orphaned more than three months and it also has been deprecated I guess it is an updated package. Hence, I use the following rule:
https://fedoraproject.org/wiki/Java#Java_on_Fedora
"... AOT compilation using GCJ has been deprecated (made optional) and new or updated packages will be built using OpenJDK to produce regular Java bytecode. ..."

I don't see any reasons to save gcj bits in the package. The F12 release doesn't include this package, therefore it is a proof that there are not any existing dependencies on the gcj bits of this package, and it can be removed without any problems. 
And also, I think, it is clear why so dangerously use of a "Java" environment that has not passed the testing via Java Compatibility Kit.
I guess we shouldn't spend the time on support of the deprecated technology.

The second release is prepared for review:
Spec URL: http://victorv.fedorapeople.org/files/jcodings.spec
SRPM URL: http://victorv.fedorapeople.org/files/jcodings-1.0.2-2.fc13.src.rpm

Comment 6 Mo Morsi 2010-02-10 18:06:03 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Even though this is in the orphaned specfile, the following line doesn't work
> > (try it yourself):
> > find -name '*.jar' -o -name '*.class' -exec rm -f '{}' \;
> > 
> > You can replace it w/ this if you want:
> > find \( -name '*.class' -o -name '*.jar' \) -exec rm -rf {} \;
> > 
> > or this:
> > find ./ -name '*.jar' -exec rm -f '{}' \;
> > find ./ -name '*.class' -exec rm -f '{}' \;
> fixed as described.
> > 
> > As far as the changelog you have a typo, "Jun 2010" should be "Jan 2010".
> fixed. 

Great, thanks for both of these.

> > 
> > Also why did you remove the previous entries from the changelog
> > http://cvs.fedoraproject.org/viewvc/rpms/jcodings/F-11/jcodings.spec?view=markup
> The clause with "find" was only a line that has not been changed in the spec.
> So, I've considered it as a new package, and a history, from my viewpoint, is
> not interesting for anybody. Nevertheless, if you prefer then I'll save the
> changelog.

Ya, think it's good to leave it in, not only for the specfile but just so we can see the package maintainer history and that it matches up w/ the cvs commit history.


> The second release is prepared for review:
> Spec URL: http://victorv.fedorapeople.org/files/jcodings.spec
> SRPM URL: http://victorv.fedorapeople.org/files/jcodings-1.0.2-2.fc13.src.rpm

Excellent, went through all the review guidelines and it fully passes.

APPROVED

Please follow http://fedoraproject.org/wiki/CVSAdminProcedure and import
the package. Close this bug as RAWHIDE once it's been successfully imported
and built.

Comment 7 Victor G. Vasilyev 2010-02-10 19:41:30 UTC
Package Change Request
======================
Package Name: jcodings
New Branches: 
Owners: mmorsi victorv
Updated Description: Java-based codings helper classes for Joni and JRuby


The package https://admin.fedoraproject.org/pkgdb/packages/name/jcodings exists, but has been orphaned and deprecated.

Comment 8 Kevin Fenzi 2010-02-11 06:20:58 UTC
cvs done.

Note that you will need to file a ticket in rel-eng track to get this package unblocked before it will build in rawhide.

Comment 9 Victor G. Vasilyev 2010-02-11 10:07:01 UTC
cvs import done.

https://fedorahosted.org/rel-eng/ticket/3376

Comment 10 Victor G. Vasilyev 2010-02-11 19:48:52 UTC
Koji build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1978297


Note You need to log in before you can comment on or make changes to this bug.