Eirik Maus commented on New Feature MAPPASM-180

Thanks for the feedback. I'll look into all of it. Sorry about the patch format. I didn't know it was non-standard. It seemed very similar. I'll create a proper one. I must admit I only tested it using the same tool that created it.

I agree that the filtering mechanism is kind of odd related to other dependency filtering mechanisms in the maven world. That is because the purpose of the filtering is somewhat different. Most other uses of filtering is applied to include or exclude some specific artifact(s). In that case, groupId and artifactId are the most important elements. Such filtering can also be done quite effectively using the dependencyManagement mechanism, so I see no point in implmenting a second mechanism for that.

In this case I'm happy with the fact that the entire runtime dependency graph is used by the plugin, but I'm not happy that dependencies that are not jar files are eliminated. I want is to have a short way to tell the plugin not to disregard dependencies of a certain type. Actually, I only need a short way to tell appassembler to stop disregarding my war-files. But I guessed that other people might have other file types they might like to include (or, rather, stopped from exclusion). It could be ear-files, it could be some kind of custom plugin-format files or whatever they would like to specify.

The other filters are sort of added on because they came for free (almost for free) at the same time. I don't know if they make any sense. Bundling the source along with the application (by classifier) could perhaps be useful, but I don't know if there is any help in letting people declare a dependency as "system" scope, and still let them include it in the application bundle. It could be the case that these features are overkill.

In any case, I agree that the proposed syntax is weird and "foreign". But it seemed to me that a more typical-maven-like syntax required huge amounts of configuration xml for the users, and, if I understood the maven plugin developers guide, introduction of plexus component(s) and what not in the appassembler plugin. The end result would probably be very flexible, but it seemed quite a lot of work for not having war-files excluded by the appassembler upon request.

I didn't know about the maven artifact filters. I'll see if they can be used instead of my own code. Perhaps it can solve the weird-syntax-issue. Lets hope they accept "::::*:war" and similar.

The unused variables are due to the javadoc annotation processing in the maven plugin development toolchain. As a pre-existing todo-remark in the code suggested, I moved the handling of the dependency filtering to a separate class. In order to retain backwards compatibility, the injection of config values was delegated to setter methods (using property="setterMethod"), that passed the argument on to the filtering class. The same goes for the new options. However, it seems like the annotation still must be present on a member variable to take effect, resulting in variables that only serve as placeholders for maven-specific javadoc tags. Thus the unused variables. I didn't write that up in the javadoc comment as is seemed like an implementation detail not suitable for public exposure (and adding a comment between the javadoc tags and the unused variable would dissociate the javadoc tags from their member variable ...). Variables that are not used in program code is indeed ugly, but at least the name suggests that is is intentional. All the integration tests run unmodified.

I guess there is still some work to do before this is finished. Any suggestions on the obstacles above would be very much appreciated.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators.
For more information on JIRA, see: http://www.atlassian.com/software/jira
--------------------------------------------------------------------- To unsubscribe from this list, please visit: http://xircles.codehaus.org/manage_email

Reply via email to