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@xxxxxxxxxx> 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review