I've uploaded
http://issues.apache.org/jira/secure/attachment/12335128/geronimo-
deployment-plugin-RTC-VOTE.2.patch
incorporating most of your comments, see below.
Thanks for the review!
david jencks
On Jun 6, 2006, at 11:57 PM, Jacek Laskowski wrote:
On 6/6/06, David Jencks <[EMAIL PROTECTED]> wrote:
Prasad has been working for a long time on an m2 deployment plugin.
I've cleaned up his latest patch a little bit and think its ready to
commit.
Please review http://issues.apache.org/jira/secure/attachment/
12335116/geronimo-deployment-plugin-RTC-VOTE.patch
I have taken a look at the patch and the comments are as follows
(they're rather style-centric nor technical, but still valid I hope).
No testing was performed.
1/ I was scared to read: "May not have been tested." in one of the
classes
You want me to lie :-) ? I don't think anyone has ever used the in-vm
startServer command in the m1 plugin, so I doubt prasad has tested
this one either. I still think its worth including as a starting
point in case some one wants to try it out.
2/ Copyright 2004-2006 - shouldn't it be Copyright 2006 only?
These are basically copies + modifications of the m1 deployment
plugin, copyright 2004. I don't know if any changes happened in
2005, but 2004 and 2006 are definitely needed.
3/ The javadoc of classes should be consistent. It means that it
should read:
/**
* @goal undeploy (if appropriate)
*
* @version $Rev$ $Date$
*/
whereas some contain
@version $Revision$ $Date$
or no version at all.
They should all have the @version, but only mojos should have the
@goal in order to not confuse maven. I've fixed the @version tags as
well as I can find them.
4/ Geronimo :: Maven Deployment Plugin using m2 -> Geronimo :: Maven 2
Deployment Plugin or alternatively Geronimo :: Maven Deployment Plugin
for Maven 2, but I'd prefer the former.
fixed
5/ geronimo-deployment-plugin/pom.xml has no ASF license header.
fixed. I think its better to include the asf license in the source
even if maven removes it during processing.
So, unless it's corrected I'm -1. If you're swampped with your other
work, I can take care of it and propose the patch corrected again.
Here's my +1.
It doesn't count, though ;-)
Why not? Because I edited prasad's patch for formatting and removed
unused cruft? Ken's directive requires 3 +1 votes from committers
other than the proposer (who apparently does not need to be a
committer): although the document he appears to have adapted states
only PMC member votes count, that is not in his directive. Since he
hasn't responded to requests for clarification I think we have to
take him at his word.
thanks
david jencks
david jencks
Jacek
--
Jacek Laskowski
http://www.laskowski.net.pl
