Details
-
Type:
Improvement
-
Status:
Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 0.1-maven2
-
Fix Version/s: 0.2-maven2
-
Component/s: Maven2
-
Labels:None
-
Environment:maven2 plugin
-
Number of attachments :
Description
Add support of starting/stopping applications in specified container through m2 plugin. Attached mojos used start-module and stop-module goal names which were suggested by Vincent Massol.
Attachments
Activity
Hide
Juri Artamonov
added a comment -
Here is updated and renamed DeployerMojos that uses new AbstractClass with common code, except DeployMojo. Deploy Mojo contains PingUrl attribute which belong to Deployable object for m2 not for real Deployable. I think this is up to you Vincent to decide if real Deployable could have getPingUrl attribute. Anyway this will be like ahack not like a fix 
Please let me know what do you think.
Best regards,
Juri.
Show
Juri Artamonov
added a comment - Here is updated and renamed DeployerMojos that uses new AbstractClass with common code, except DeployMojo. Deploy Mojo contains PingUrl attribute which belong to Deployable object for m2 not for real Deployable. I think this is up to you Vincent to decide if real Deployable could have getPingUrl attribute. Anyway this will be like ahack not like a fix
Please let me know what do you think.
Best regards,
Juri.
Please let me know what do you think.
Best regards,
Juri.
Hide
Vincent Massol
added a comment -
Hi Juri. Thanks a lot for your patch! I've applied it. Here are the things I had to change. I'm just listing them so that you know what can be improved for the next time you send some patches. Some of them are important and others are minor but I'm listing them all in order to be exhaustive:
- The patch is not created from the right directory. For example in the patch file it says:
D:/workspace/cargo-svn/extensions/maven2/src/main/java/org/codehaus/cargo/maven2/ModuleStopMojo.java
The root of the patch should be extensions/. I've fixed this manually for now but please make sure this is fine for the next patches you send.
- The copyright year should be the year when the file was created till the last time it was modified. Thus new files should have a copyright of 2006.
- The @version is not correct. It must be: "@version $Id: $" so that the SVN client replaces the value when the file is committed. For example you'll get:
@version $Id: AbstractCargoMojo.java 940 2006-03-17 21:29:46Z vmassol $
- Your patch should include deleted files too. It did not include deletion of currnet DeployMojo/UndeployMojo
- There are several blank lines that shouldn't exist as in:
private void deploySingleDeployable(org.codehaus.cargo.container.deployer.Deployer deployer, org.codehaus.cargo.container.deployable.Deployable deployable, URL pingURL) { getLog().debug("Deploying [" + deployable.getFile() + "]" + ((pingURL == null) ? "..." : " using ping URL [" + pingURL + "]")); if (pingURL != null) { deployer.deploy(deployable, createDeployableMonitor(pingURL, deployable)); } else { deployer.deploy(deployable); } }
(in this example you can remove 4 lines).
- Method shoud be verbs. I've replaced moduleAction() by performDeployerAction(). (Note: I make this mistake myself but this doesn't mean someone shouldn't correct me! )
- I've replaced all Module names with Deployer instead so that it's easier to relate code to the Cargo API. Module is a term that doesn't exist in cargo. Right now as I've suggetsed on the mailing list (I don't think anyone answered) I had suggested deployer-start, deployer-deploy, deployer-undeploy, deployer-stop, which I think is better than module-start, etc.
- Added some comments to the code
- Refactored more (execute in parent class, removed duplications in deploy and undeploy mojos). The pingURL should be passed to all. In the future there should be APIs in the Deployer interface for using them when starting and stopping deployables too.
- Add redeploy mojo to fully implement the Deployer interface
- Changed goal name. I'm happy to change it back to what you defined or whatever we all agree on. I've sent again a mail on the cargo dev list for feedback.
Once more thanks a lot for your patch!
Show
Vincent Massol
added a comment - Hi Juri. Thanks a lot for your patch! I've applied it. Here are the things I had to change. I'm just listing them so that you know what can be improved for the next time you send some patches. Some of them are important and others are minor but I'm listing them all in order to be exhaustive:
(in this example you can remove 4 lines).
- The patch is not created from the right directory. For example in the patch file it says:
- The copyright year should be the year when the file was created till the last time it was modified. Thus new files should have a copyright of 2006.
- The @version is not correct. It must be: "@version $Id: $" so that the SVN client replaces the value when the file is committed. For example you'll get:
- Your patch should include deleted files too. It did not include deletion of currnet DeployMojo/UndeployMojo
- There are several blank lines that shouldn't exist as in:
private void deploySingleDeployable(org.codehaus.cargo.container.deployer.Deployer deployer, org.codehaus.cargo.container.deployable.Deployable deployable, URL pingURL) { getLog().debug("Deploying [" + deployable.getFile() + "]" + ((pingURL == null) ? "..." : " using ping URL [" + pingURL + "]")); if (pingURL != null) { deployer.deploy(deployable, createDeployableMonitor(pingURL, deployable)); } else { deployer.deploy(deployable); } }
- Method shoud be verbs. I've replaced moduleAction() by performDeployerAction(). (Note: I make this mistake myself but this doesn't mean someone shouldn't correct me! )
- I've replaced all Module names with Deployer instead so that it's easier to relate code to the Cargo API. Module is a term that doesn't exist in cargo. Right now as I've suggetsed on the mailing list (I don't think anyone answered) I had suggested deployer-start, deployer-deploy, deployer-undeploy, deployer-stop, which I think is better than module-start, etc.
- Added some comments to the code
- Refactored more (execute in parent class, removed duplications in deploy and undeploy mojos). The pingURL should be passed to all. In the future there should be APIs in the Deployer interface for using them when starting and stopping deployables too.
- Add redeploy mojo to fully implement the Deployer interface
- Changed goal name. I'm happy to change it back to what you defined or whatever we all agree on. I've sent again a mail on the cargo dev list for feedback.
Very cool, thanks Juri!
Some remarks: