[Bug 706984] New: Review Request: args4j-site - Parent POM file for args4j

[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: args4j-site - Parent POM file for args4j

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

Summary: Review Request: args4j-site - Parent POM file for
                    args4j
           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
      Story Points: ---

Spec URL: http://jcapik.fedorapeople.org/files/args4j-site/1/args4j-site.spec
SRPM URL:
http://jcapik.fedorapeople.org/files/args4j-site/1/args4j-site-2.0.16-1.fc16...
Description: 
This package contains the parent pom file for args4j projects.

[Bug 706984] Review Request: args4j-site - Parent POM file for args4j

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

Jaromír Cápík  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |652183(FE-JAVASIG)

[Bug 706984] Review Request: args4j-site - Parent POM file for args4j

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

Alexander Kurtakov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Alexander Kurtakov  2011-05-23 13:12:03 EDT ---
Why just the pom but not the whole project?
Looks like all the sources are in the same tag that has been used for fetching
the pom.

[Bug 706984] Review Request: args4j-site - Parent POM file for args4j

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

--- Comment #2 from Alexander Kurtakov  2011-05-24 02:09:34 EDT ---
svn export https://svn.java.net/svn/args4j~svn/tags/args4j-site-2_0_16

removing wagon-svn extension from the main pom.xml
fix ant group to be org.apache.ant

everything builds fine

[Bug 706984] Review Request: args4j-site - Parent POM file for args4j

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

--- Comment #3 from Alexander Kurtakov  2011-05-24 02:10:22 EDT ---
I would really prefer to see one package for the whole args4j.

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

Jaromír Cápík  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Review Request: args4j-site |Review Request: args4j -
                   |- Parent POM file for       |Small Java lib that makes
                   |args4j                      |it easy to parse command
                   |                            |line options/args in CUI
                   |                            |apps

--- Comment #4 from Jaromír Cápík  2011-05-24 12:15:01 EDT ---
Reworked ...

Spec URL: http://jcapik.fedorapeople.org/files/args4j/args4j.spec
SRPM URL:
http://jcapik.fedorapeople.org/files/args4j/args4j-2.0.16-1.fc14.src.rpm

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

--- Comment #5 from Jaromír Cápík  2011-05-25 05:16:43 EDT ---
I'm not sure about the ant runtime dependency for args4j-tools. Source files do
not seem to use it.

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

--- Comment #6 from Jaromír Cápík  2011-05-25 07:46:55 EDT ---
ant dependency removed from the args4j-tools pom.

Spec URL: http://jcapik.fedorapeople.org/files/args4j/2/args4j.spec
SRPM URL:
http://jcapik.fedorapeople.org/files/args4j/2/args4j-2.0.16-2.fc14.src.rpm

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

Marek Goldmann  changed:

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

--- Comment #7 from Marek Goldmann  2011-05-26 12:00:14 EDT ---
I'll take this one.

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

--- Comment #8 from Marek Goldmann  2011-05-26 12:38:55 EDT ---
Just a few small comments:

Key:
- = N/A
x = Check
! = Problem
? = Not evaluated

=== REQUIRED ITEMS ===
[x]  Rpmlint output:

$ rpmlint args4j-2.0.16-2.fc14.src.rpm
args4j.src: I: enchant-dictionary-not-found en_US
args4j.src: W: invalid-url Source0: args4j-2.0.16.tar.xz
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

[x]  Package is named according to the Package Naming Guidelines[1].
[x]  Spec file name must match the base package name, in the format
%{name}.spec.
[x]  Package meets the Packaging Guidelines[2].
[x]  Package successfully compiles and builds into binary rpms.
[x]  Buildroot definition is not present
[x]  Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging
Guidelines[3,4].
[!]  License field in the package spec file matches the actual license.

Which part of the lib is licensed under BSD? Homepage states only MIT.

[x]  If (and only if) the source package includes the text of the license(s) in
its own file, then that file, containing the text of the license(s) for the
package is included in %doc.
[x]  All independent sub-packages have license of their own
[x]  Spec file is legible and written in American English.
[!]  Sources used to build the package matches the upstream source, as provided
in the spec URL.
MD5SUM this package    :
MD5SUM upstream package:

It seems the instructions to get the source code from SVN are not working for
me. Could you please check it?

[x]  All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines[5].
[x]  Package must own all directories that it creates.
[-]  Package requires other packages for directories it uses.
[x]  Package does not contain duplicates in %files.

Contains license in subpackage, but this is OK.

[x]  Permissions on files are set properly.
[x]  Package does NOT have a %clean section which contains rm -rf %{buildroot}
(or $RPM_BUILD_ROOT). (not needed anymore)
[x]  Package consistently uses macros (no %{buildroot} and $RPM_BUILD_ROOT
mixing)
[x]  Package contains code, or permissable content.
[x]  Fully versioned dependency in subpackages, if present.
[-]  Package contains a properly installed %{name}.desktop file if it is a GUI
application.
[x]  Package does not own files or directories owned by other packages.
[x]  Javadoc documentation files are generated and included in -javadoc
subpackage
[x]  Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlinks)
[x]  Packages have proper BuildRequires/Requires on jpackage-utils
[x]  Javadoc subpackages have Require: jpackage-utils
[x]  Package uses %global not %define
[!]  If package uses tarball from VCS include comment how to re-create that
tarball (svn export URL, git clone URL, ...)

Instructions doesn't work.

[!]  If source tarball includes bundled jar/class files these need to be
removed prior to building

Please add rm -rf args4j/lib

[x]  All filenames in rpm packages must be valid UTF-8.
[x]  Jar files are installed to %{_javadir}/%{name}.jar (see [6] for details)
[x]  If package contains pom.xml files install it (including depmaps) even when
building with ant
[x]  pom files has correct add_to_maven_depmap call which resolves to the pom
file (use "JPP." and "JPP-" correctly)

=== Maven ===
[x]  Use %{_mavenpomdir} macro for placing pom files instead of
%{_datadir}/maven2/poms
[-]  If package uses "-Dmaven.test.skip=true" explain why it was needed in a
comment
[-]  If package uses custom depmap "-Dmaven2.jpp.depmap.file=*" explain why
it's needed in a comment
[x]  Package uses %update_maven_depmap in %post/%postun
[x]  Packages have Requires(post) and Requires(postun) on jpackage-utils (for
%update_maven_depmap macro)

=== Other suggestions ===
[x]  If possible use upstream build method (maven/ant/javac)
[x]  Avoid having BuildRequires on exact NVR unless necessary
[x]  Package has BuildArch: noarch (if possible)
[!]  Latest version is packaged.

I don't know :) The download section on homepage gives 404. Any hints?

[x]  Reviewer should test that the package builds in mock.

http://koji.fedoraproject.org/koji/taskinfo?taskID=3093982

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

--- Comment #9 from Jaromír Cápík  2011-05-30 09:22:31 EDT ---
Hi Marek.

I somehow haven't noticed You've done the review :]
> > Which part of the lib is licensed under BSD? Homepage states only MIT.

LICENSE.txt contained in the source tarball is BSD and the project homepage
states MIT. I'll try to reach upstream again, but as I got no answer for my
first question, I don't suppose I'll be more successful in case of license
clarification -> don't know if we should wait for the clarification.
> > It seems the instructions to get the source code from SVN are not working for

me. Could you please check it?

You have to register on java.net and upload Your personal ssh key to the server
in order to be able to export the sources. It seems that it would be better to
make a note in the spec file stating this fact.
> > I don't know :) The download section on homepage gives 404. Any hints?

Yes. They don't care about the link anymore. The only way how to obtain the
source is via the svn export.

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

--- Comment #10 from Jaromír Cápík  2011-05-30 09:38:30 EDT ---
bundled lib dir removed + note about the ssh key added:

Spec URL: http://jcapik.fedorapeople.org/files/args4j/3/args4j.spec
SRPM URL:
http://jcapik.fedorapeople.org/files/args4j/3/args4j-2.0.16-3.fc14.src.rpm

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

Marek Goldmann  changed:

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

--- Comment #11 from Marek Goldmann  2011-05-30 11:38:27 EDT ---
[x]  License field in the package spec file matches the actual license.

OK, thanks for clarification, specify both licenses then.

[!]  Sources used to build the package matches the upstream source, as provided
in the spec URL.
MD5SUM this package    : b765c9c25789884cb982e7c8fefc0de0
MD5SUM upstream package: b29f50e3dd5933fb7fe498a38f9fb191

It seems they way you create the package changes the timestamps. Use xz
compression method - change the instructions (and update the file in srpm) to:

# tar cafJ args4j-2.0.16.tar.xz args4j-2.0.16

[x]  If source tarball includes bundled jar/class files these need to be
removed prior to building

Thanks!

[x]  Latest version is packaged.

================
*** APPROVED ***
================

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

Marek Goldmann  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|652183(FE-JAVASIG)          |

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

--- Comment #12 from Jaromír Cápík  2011-05-31 05:33:13 EDT ---
Hi Marek.

The differences are caused by the svn export, since it produces different
timestamps of the directories. That's why MD5SUM cannot be used in this case.
Only the directory diff is helpful.

BR, J.

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

Jaromír Cápík  changed:

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

--- Comment #13 from Jaromír Cápík  2011-05-31 10:24:15 EDT ---
New Package SCM Request
=======================
Package Name: args4j
Short Description: Small Java lib that makes it easy to parse command line
options/args in CUI apps
Owners: jcapik
Branches: f15
InitialCC: java-sig

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

--- Comment #14 from Jason Tibbitts  2011-05-31 15:15:54 EDT ---
Git done (by process-git-requests).

[Bug 706984] Review Request: args4j - Small Java lib that makes it easy to parse command line options/args in CUI apps

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

Jaromír Cápík  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE
        Last Closed|                            |2011-06-08 12:21:10

--- Comment #15 from Jaromír Cápík  2011-06-08 12:21:10 EDT ---
The package has been built successfuly:
https://koji.fedoraproject.org/koji/buildinfo?buildID=246243

Thanks for the review and git repo.

Closing.