[Bug 606720] Review Request: maven-shade-plugin - This plugin provides the capability to package the artifact in an uber-jar

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

Stanislav Ochotnicky <sochotni at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|needinfo?(sochotni at redhat.c |
                   |om)                         |

--- Comment #5 from Stanislav Ochotnicky <sochotni at redhat.com> 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.