Comments below... get ready to take your grain of salt...
On Jul 25, 2006, at 6:50 AM, anita kulshreshtha wrote:
Jason,
you aksed me on to make a patch for the branch on July 10th. I
looked at the code and saw that you made the following changes to the
packaging plugin:
1. Renamed the plugin
2. Changed the package structure
Yes I merged the 2 plugins into one so they can share the same
codebase. These plugins work with CAR files... and there is
absolutely no reason whatsoever that they should or need to be
separated. There are however good reasons to merge them.
3. Changed the directory layout to standard M2 layout, even though the
rest of the project and the geronimo-deployment _plugin_ still use the
old layout!
There are several modules in the project that use the new standard M2
layout. I moved all modules that were not used by the m1 build to
use the new layout and others will follow shortly.
I don't really see how this matters though. It should be easy enough
for the author of the patch to simply copy and paste the correct bits
in the right place. The code has not changed so much that the
original logic was removed.
4. Used non geronimo coding style.
In the past subjective changes like this were discussed on the
list
so people can provide their input.
Please don't tell me about coding style... the classes in these
plugins were a mix of styles, a mix indentation, grossly abused
System.out.* and did not properly handle exceptions. I normalized them.
I also make may plexus-style changes, which better use the plexus
component container's ability to configure the mojos, that and to
take more advantage of existing plexus bits like utils to delete
directories instead of your custom version.
5. Removed plexus component.xml (you have reverted this change now)
At no point did I remove plexus/components.xml. I am very well aware
of the purpose of this file.
If you are going to publicly list reasons you don't like what I have
done, please do not invent reasons that do not exist, or substantiate
them.
I quickly realized that it was going to require lot more effort to
create the patch. If I had the time, I would have created the patch
for
you. July is not the most productive time for me. The schools are off
and I must attend to other children's activities. IMO, the fastest way
to get this working will be for you to add the code from the
m2-plugins.patch to your code. If you need the java files, I will be
happy to upload them.
I have been reviewing that patch and have started to incorporate the
parts which I believe are relevant. I saw you added comments about
the use-case to a JIRA which I will also review.
Once again I will request you to use ${j2eeJettyServer} etc for
deploymentConfig. If it was up to me I would choose ease of use over
Maven's preferred ways. This plugin is used by many users who
really do
not care to know about the deployers let alone their ordering. As far
as they are concerned they want to specify which custom server they
are
packaging for. As we provide more custom servers we will be adding new
properties.
If it was up to me, I'd remove the need for these plugins altogether
and deploy xml directly to the server and hide all of this the CAR
non-sense from our users.
Anyways, I disagree that in the previous ${...} state that users
don't have to worry about the ordering... since they need to get that
property from somewhere. In either case, something is going to get
copy-pasted. If it really turns out to be a huge user burden then I
would rather investigate a cleaner way to handle this configuration
which hides more of the complexity from users.
--jason
