[Bug 665544] Review Request: ini4j - Java API for handling files in Window .ini format

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

--- Comment #5 from Omair Majid <omajid at redhat.com> 2011-01-13 14:36:07 EST ---
Thanks for the review! Updated files:

Spec URL: http://omajid.fedorapeople.org/ini4j/ini4j.spec
SRPM URL: http://omajid.fedorapeople.org/ini4j/ini4j-0.4.1-3.fc15.src.rpm

(In reply to comment #4)
> ini4j-javadoc.noarch: W: spelling-error Summary(en_US) doccumentation ->
> documentation, instrumentation, argumentation
> "cc"->"c"

Fixed.

> [x]  Packages have proper BuildRequires/Requires on jpackage-utils
> Although javadoc subpackage has Requires both on main package and
> jpackage-utils. If you don't have strong reason to depend on main package it
> might be good idea to drop that dependency. If you decide to keep main package
> Requires then jpackage-utils is not necessary since main package already
> requires it.
> 

The updated spec file removes the dependency on main package. Does the -javadoc
subpackage also need a copy of the license then?

> [!]  Jar files are installed to %{_javadir}/%{name}.jar (see [6] for details)
> Please no -%{version}.jar files. Current guidelines have versionless jars.

Fixed.

> [!]  If package uses "-Dmaven.test.skip=true" explain why it was needed in a
> comment
> If it's possible prefer to use -Dmaven.test.failure.ignore=true instead of
> test.skip. This is useful if tests compile but fail to run correctly. We can
> still see the output in the build.log. In both cases please explain why
> test.skip was used so that it can be removed in the future when the reason is
> gone.
> 

The tests fail to compile since they require EasyMock Class Extension 2.3
(http://www.easymock.org/EasyMock2_3_ClassExtension_Documentation.html)

> [!]  If package uses custom depmap "-Dmaven2.jpp.depmap.file=*" explain why
> it's needed in a comment
> Removing activation is OK, but please file a bug for plexus-mail-sender to
> provide correct depmap. I'll work on fixing it in the meantime :-)

Filed as bug 669495.

> [!]  Packages have Requires(post) and Requires(postun) on jpackage-utils (for
> %update_maven_depmap macro)
> You are missing proper requires post/postun on jpackage-utils

Fixed.

Thanks again for the review.

-- 
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.