Maven 1.x Announcement Plugin

Hi Vincent,

As promised, here is a patch for this issue. It adds too new goals (mail and mail-all) and requires a couple of properties.

Hava fun,

Felipe

We have to consider adding a mailing list address to next version of POM

Yep, agreed.

That was my first thought for a default mailTo field, but the POM only provides subscribe and unsubscribe addresses.

Hi Felipe,

Thanks! Some comments:

• What is the use case for the mail-all goal?
• I have put the public properties in plugin.properties
• I have have introduced %VERSION% template in subject property
• javamail jar not on ibiblio... We cannot release the plugin. We need to find an alternative jar
• Used maven:param-check instead of manual checks

(attached new patch)

Patch to your patch

Hi Vincent,

Some COC (comments on comments inline:

> What is the use case for the mail-all goal?

Consistency. If there is a generate-all, why not a mail-all? I created just the mail originally, but then when I wrote the full patch (i.e., goals.xml, properties.xml, etc..) I realized there was a generate-all, so it might make sense to have a mail-all too.

> - I have put the public properties in plugin.properties

Sorry, I forgot that one

>- I have have introduced %VERSION% template in subject property
Cool. That would solve the problem of resolving the ${versionVariable} on mail-all at 'runtime' . In fact, my first idea to the problem was using such a variable replacement schema, but I didn't know it was a common practice (on Maven plugins) and was afraid the final result would be too complex (guess next time I should be asking these questions in the dev list first

> - javamail jar not on ibiblio... We cannot release the plugin. We need to find an alternative jar
Initially, I thought that could be an issue too, so I checked what the jelly-tag-email does, and they use javamail. But looks like they have the same issue, as there is no official release yet. Do you know any replacement? After a quick look on ibiblio, the closest I found was james, which is ASF's Java-bases email server (as they use ASL, they might use an alternative for JavaMail)

> - Used maven:param-check instead of manual checks
Cool, I didn't know that trick.

PS: regarding the replace() function, it would be nice if it belonged to the Jelly util tag - they already have a replace tag that works only with chars. I will try to make such a patch and submit to them.

Regarding javamail's replacement, I think the best is to use commons-net as suggested by Maury on the maven dev ML.

For the replace() method, yes, it would be nice to have it available in jelly util code. That said, once we all move to JDK 1.4, it won't be needed anymore as it's built in the JDK.

For the replace() method - does the util:replace tag help?

dIon, no it doesn't. The replace tag replaces only a single char:

String stringAnswer = answer.toString().replace(oldChar.charAt(0), newChar.charAt(0));

It would be nice to replace the existing replace jelly tag with the following replace implementation:

/**

• @param original the original template
• @param oldPattern the pattern to replace
• @param newPattern the new pattern
• @return the modified template string with patterns applied
  */
  public String replace(String original, String oldPattern,
  String newPattern)
  {
  int index, oldIndex;
  StringBuffer buffer = new StringBuffer();

if((index = original.indexOf(oldPattern)) != -1) {
oldIndex = 0;
while((index = original.indexOf(oldPattern, oldIndex)) != -1) { buffer.append(original.substring(oldIndex, index)); buffer.append(newPattern); oldIndex = index + oldPattern.length(); }
buffer.append(original.substring(oldIndex));
original = buffer.toString();
}
return original;
}

Refactoring on Vincent's patch using Jakarta Commons Lang (let's not reinvent the wheel for StringUtils

New patch, using commons-net instead of jelly-email.

Oops, looks like 'cvs diff -u' does not add new files...

After a few more tests, I'm sending a new patch (this is the last one, I promise , with the following improvements:

• better handling on network exceptions
• removal of CC: (as commons-net CC doesn't work properly)

Notice that I've packed the patch and the new Java file in one zip (cvs diff -u would only work if I could do a cvs add first).

Felipe

You can do cvs add foo/bar.java, it's only a client side operation. You won't be able to commit the file though. This will as you said make cvs diff include the new files.

> You can do cvs add foo/bar.java, it's only a client side operation.

That's what I thought too, but it didn't work:

[felipeal@localhost]~/cvs/maven/maven-plugins/announcement: cvs add src/main/org/apache/maven/announcement/MailUtils.java
cvs [server aborted]: "add" requires write access to the repository

I couldn't send a message from my SMTP server at work because I wasn't sending the HELO message.
So, here is yet another patch, this time using a maven property to set the host to be used in the HELO message (which by default is localhost)

just on new files and CVS:

cvs diff -uN

should do the trick

> cvs diff -uN
> should do the trick

Yes, it would work only if I had 'cvs added' the files first, but unfortunately that didn't work neither (see previous post).

Applied. Thanks! One little comment. I think it would have been best if the sendMail() method throws Exception instead of returning status strings. The Jelly code can then use the <j:catch> tag to catch the exceptions and display an error message.

Hi Vincent,

The first patch using commmons-net was using that approach, but I changed it to a return code for some reason - I guess I wasn't using the <j:catch> and then Maven was throwing those nasty exceptions.

I will change it and a provide a new patch later (I don't have a maven environment where am I now).

Felipe

Vincent,

I changed the code to throw an exception and then I figured it out why I have chosen to return a String instead: the reason is that j:invokeStatic wrapps the original expception, so the <j:catch> would return something like this:

Could not send message. Reason: /home/felipeal/.maven/cache/maven-announcement-plugin-1.3-SNAPSHOT/plugin.jelly:159:94: <j:invokeStatic> Could not load class: org.apache.maven.announcement.MailUtils. Reason: java.lang.reflect.InvocationTargetException

So, we better leave the code the way it is.

– Felipe

Felipe,

What if we use <j:useBean> instead? Does it also wrap the exception? Otherwise, no worry.