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=606720 Stanislav Ochotnicky <sochotni@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(sochotni@xxxxxxxx | |om) | --- Comment #5 from Stanislav Ochotnicky <sochotni@xxxxxxxxxx> 2010-07-08 07:57:38 EDT --- First: Thanks for great (albeit unofficial) review so far. 1. jar files present in the tarball are used for testing. Notable exception was plexus-util that I replaced with symlink to our own plexus-util (fixed in this version) Rest of the jars are not installed they are just small test cases. 2. Hmmm, perhaps you misread the spec file? This is what I see in mine: %package javadoc Group: Documentation Summary: API documentation for %{name} Requires: jpackage-utils %description javadoc %{summary}. 3. I actually didn't use macro commands anywhere in the spec file AFAIK. I prefer it this way. I don't see a reason to use %__mkdir_p when "mkdir -p" behaves consistently across all implementations I know of... 4. I didn't create that dir, because maven creates it itself if it doesn't exist. But you're right...it's better to create it. 5. Exactly because MAVEN_REPO_LOCAL is not used anywhere besides %build section, defining %global seems too much. Maybe replacing with local %define in %build section, but still...it's a question of style and this seems like the most common style used in Fedora spec files. 6. I am sorry I didn't warn in advance...this package was prepared only for dist-f14-maven221 tag, Currently it builds fine in rawhide (maven221 tag was merged recently...so maybe that's why it didn't work for you back then). See my build on koji if you want (s)rpms 7. Fixed 8. Fixed 9. This should not be needed, but apparently in the past there were some tools that had problems with unspecified Epochs. Some considered them -1 and some 0. Just to be on the safe side, Epoch 0 was used. This is just a precaution, but doesn't actually affect anything else beside upgrades from older maven. Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2304028 Spec file: http://sochotni.fedorapeople.org/maven-shade-plugin.spec SRPM: http://sochotni.fedorapeople.org/maven-shade-plugin-1.3.3-2.fc13.src.rpm -- 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