Last (and only, depending on how you count) +1 was 4 days ago, any
chance 2 more people are willing to review this?
thanks
david jencks
On Jun 9, 2006, at 7:51 AM, Guillaume Nodet wrote:
+1
Cheers,
Guillaume Nodet
David Jencks wrote:
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
