Bug 227061 - Review Request: isorelax-0.1-0.20041111.2jpp - Public interfaces useful for applications to support RELAX Core
Review Request: isorelax-0.1-0.20041111.2jpp - Public interfaces useful for a...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Vivek Lakshmanan
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-02-02 12:37 EST by Rafael H. Schloming
Modified: 2014-12-01 18:13 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-06 18:09:36 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
dbhole: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)
This should clear up the issues with the review. (5.27 KB, patch)
2007-02-12 16:28 EST, Andrew Overholt
no flags Details | Diff

  None (edit)
Description Rafael H. Schloming 2007-02-02 12:37:28 EST
Spec URL: http://people.redhat.com/rafaels/specs/isorelax-0.1-0.20041111.2jpp.spec
SRPM URL: ftp://jpackage.hmdc.harvard.edu/JPackage/1.7/generic/SRPMS.free/isorelax-0.1-0.20041111.2jpp.src.rpm
Description: The ISO RELAX project is started to host the public interfaces
useful for applications to support RELAX Core. But nowadays
some of the stuff we have is schema language neutral.

Javadoc for isorelax.
Comment 1 Andrew Overholt 2007-02-12 11:35:30 EST
I'll take this one.
Comment 2 Andrew Overholt 2007-02-12 16:22:43 EST
Things marked with an X need to be fixed.

MUST:
* package is named appropriately
* it is legal for Fedora to distribute this
* license field matches the actual license.
* license is open source-compatible.
X specfile name matches %{name}
 . the specfile needs to be isorelax.spec
X verify source and patches
 . we need to add the following:
  # mkdir isorelax-release-20050331-src
  # cd isorelax-release-20050331-src
  # cvs -d:pserver:anonymous@iso-relax.cvs.sourceforge.net:/cvsroot/iso-relax \
  #   export -r release-20050331 src lib
  # cvs -d:pserver:anonymous@iso-relax.cvs.sourceforge.net:/cvsroot/iso-relax \
  #   co -r release-20050331 build.xml
  # cd ..
  # tar cjf isorelax-release-20050331-src.tar.bz2 isorelax-release-20050331-src
X the description should be fixed to not be from the author's point of view
X correct buildroot
 - should be:
   %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 - this won't hold up the review, though, as there's currently a discussion
   regarding buildroots going on
X release tag
 . we need to fix the release tag to be of the form
   0.Z.<tag>.Xjpp.Y%{?dist}
X license text included in package and marked with %doc
 . upstream does not include their license in CVS
* packages meets FHS (http://www.pathname.com/fhs/)
X rpmlint on isorelax srpm gives this output

W: isorelax non-standard-group Development/Libraries/Java

. can be ignored

W: isorelax unversioned-explicit-obsoletes isorelax-bootstrap
W: isorelax unversioned-explicit-provides isorelax-bootstrap

. I think we should just remove those virtual obsoletes/provides as they've
  never been shipped in Fedora.

W: isorelax setup-not-quiet

. I think it's the cat.  That should just be in a comment, I think.

E: isorelax no-cleaning-of-buildroot %install

. add rm -rf $RPM_BUILD_ROOT to the beginning of %install

W: isorelax mixed-use-of-spaces-and-tabs (spaces: line 9, tab: line 38)

. the easiest way to fix this is to run with emacs and do M-x untabify

* changelog is in acceptable format
* Packager tag should not be used
X Vendor tag should not be used
 . remove Vendor
 . remove Distribution
* use License and not Copyright 
* Summary tag does not end in a period
* no PreReq
* specfile is legible
* package successfully compiles and builds on at least x86
? BuildRequires are proper
 . I'm not sure about this one.  I guess we should verify if one of the
   packages that BRs this builds okay.
* summary should be a short and concise description of the package
* description expands upon summary (don't include installation
instructions)
* make sure lines are <= 80 characters
* specfile written in American English
* make a -doc sub-package if necessary
* no static libraries
* no rpath
* no config files
* not a GUI app
* no need for a -devel sub-package?
* macros used appropriately and consistently
* no locale data
* package is not relocatable
* package contains code
* package owns all directories and files
* no %files duplicates
* file permissions okay; %defattrs present
* %clean is present
* %doc files do not affect runtime
* not a webapp
* verify the final provides and requires of the binary RPMs
* final provides and requires are sane:

X rpmlint on the binary RPMs:
 . package doesn't build on i386

11. ERROR in
/home/andrew/rpmbuild/BUILD/isorelax-0.1/src/org/iso_relax/jaxp/ValidatingDocumentBuilderFactory.java
(at line 15)
    public class ValidatingDocumentBuilderFactory extends DocumentBuilderFactory
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The type ValidatingDocumentBuilderFactory must implement the inherited abstract
method DocumentBuilderFactory.setFeature(String, boolean)
----------
12. ERROR in
/home/andrew/rpmbuild/BUILD/isorelax-0.1/src/org/iso_relax/jaxp/ValidatingDocumentBuilderFactory.java
(at line 15)
    public class ValidatingDocumentBuilderFactory extends DocumentBuilderFactory
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The type ValidatingDocumentBuilderFactory must implement the inherited abstract
method DocumentBuilderFactory.getFeature(String)

SHOULD:
X package should include license text in the package and mark it with %doc
 . upstream does not do this
X package should build on i386
 . nope (see above)
X package should build in mock
 . didn't try
Comment 3 Andrew Overholt 2007-02-12 16:28:35 EST
Created attachment 147939 [details]
This should clear up the issues with the review.
Comment 4 Andrew Overholt 2007-02-12 16:40:39 EST
Fixed spec and SRPM:

> ? BuildRequires are proper
>  . I'm not sure about this one.  I guess we should verify if one of the
>    packages that BRs this builds okay.

I still think this should be done.

> X rpmlint on the binary RPMs:
>  . package doesn't build on i386

Fixed.
Comment 5 Andrew Overholt 2007-02-12 17:13:31 EST
I've verified that it builds in mock.

Fixed spec and SRPM:

http://www.overholt.ca/fedora/isorelax.spec
http://www.overholt.ca/fedora/isorelax-0-0.1.release20050331.1jpp.1.src.rpm
Comment 6 Deepak Bhole 2007-02-12 17:56:04 EST
Most of it is okay. I found the following issues:

- License file is not present in the rpm, it should be, and marked %doc
- javadoc directory should be marked %doc
- Line 5 in %install is > 80 characters
Comment 7 Andrew Overholt 2007-02-13 10:51:36 EST
(In reply to comment #6) 
> - License file is not present in the rpm, it should be, and marked %doc

The package doesn't contain its licence.  I don't think we should be adding it.

> - javadoc directory should be marked %doc

Fixed.

> - Line 5 in %install is > 80 characters

Fixed.

Updated SRPM, spec, and binary RPMs:

http://www.overholt.ca/fedora/isorelax.spec
http://www.overholt.ca/fedora/isorelax-0-0.1.release20050331.1jpp.1.src.rpm
http://www.overholt.ca/fedora/isorelax-0-0.1.release20050331.1jpp.1.noarch.rpm
http://www.overholt.ca/fedora/isorelax-javadoc-0-0.1.release20050331.1jpp.1.noarch.rpm
Comment 8 Deepak Bhole 2007-02-13 11:08:10 EST
Fixes verified. Approved.

Let me know when this is built in Brew and I will mark it closed.
Comment 9 Vivek Lakshmanan 2007-03-02 18:43:25 EST
New Package CVS Request
=======================
Package Name: isorelax
Short Description: Public interfaces for RELAX Core
Owners: vivekl@redhat.com
Branches: devel
InitialCC: vivekl@redhat.com,dbhole@redhat.com
Comment 10 Josh Boyer 2007-03-02 19:46:17 EST
(In reply to comment #9)
> New Package CVS Request
> =======================
> Package Name: isorelax
> Short Description: Public interfaces for RELAX Core
> Owners: vivekl@redhat.com
> Branches: devel
> InitialCC: vivekl@redhat.com,dbhole@redhat.com

Do you want dbhole@redhat.com to be a co-maintainer or just on initialCC?  And
the owner doesn't need to be listed in initialCC

Comment 11 Vivek Lakshmanan 2007-03-02 19:58:38 EST
(In reply to comment #10)

> Do you want dbhole@redhat.com to be a co-maintainer or just on initialCC?  
Just on initial CC please
> And the owner doesn't need to be listed in initialCC
Ah wasnt quite clear from http://fedoraproject.org/wiki/CVSAdminProcedure
I will refrain from adding it in the future. 
Thanks!

Comment 12 Vivek Lakshmanan 2007-03-06 18:09:36 EST
Built. Should be visible in rawhide soon.
Comment 13 Orion Poplawski 2010-01-08 11:23:50 EST
This builds in EL-5 - could we get a branch there?
Comment 14 Mat Booth 2011-08-07 16:56:07 EDT
Package Change Request
======================
Package Name: junitperf
New Branches: el5 el6
Owners: mbooth
Comment 15 Jon Ciesla 2011-08-08 06:03:54 EDT
Git done (by process-git-requests).
Comment 16 Jochen Schmitt 2011-11-29 12:17:08 EST
Package Change Request
======================
Package Name: isorelax
New Branches: el6
Owners: s4504kr

Original package owner should commit this is ok, but it seems that the package owner is MIA.
Comment 17 Jon Ciesla 2011-11-29 12:25:42 EST
Git done (by process-git-requests).
Comment 18 Mat Booth 2011-11-29 13:23:18 EST
You only send your email 2 days ago. I was away for the weekend. ;-)

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