Details
-
Type:
Bug
-
Status:
Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: None
-
Fix Version/s: None
-
Labels:None
-
Environment:version: 1.0-SNAPSHOT
svn rev: 2121
-
Number of attachments :4
Description
Most of the mojo parameters of the retrotranslator plugin for maven-2 do not have descriptions. Also, no variables are defined for them to allow for setting their values from command line.
-
- retrotrans-plgn.doc-n-var-params.MOJO-459.patch
- 14/Jul/06 3:55 AM
- 3 kB
- Anagnostopoulos Kostis
-
- retrotrans-plgn.doc-n-var-params.MOJO-459-2.patch
- 03/Nov/06 5:03 AM
- 2 kB
- Anagnostopoulos Kostis
-
- retrotrans-plgn.doc-n-var-params.MOJO-459-2.patch
- 15/Jul/06 7:33 PM
- 3 kB
- Anagnostopoulos Kostis
-
- retrotrans-plgn.doc-n-var-params.MOJO-459-2.patch
- 15/Jul/06 6:56 PM
- 3 kB
- Anagnostopoulos Kostis
Activity
I'm not very found of the ${maven.* bits
These mean that those properties must be set for the plugin to function, or the user will be forced to override each configuration element.
I'm happy to commit the docs, but not the ${maven.* bits
Feel free to comment.
Documentation bits committed
${maven.* bits not committed; if you really want that added, then please explain more why
Hi Jason,
I can't think of a reason why "the user will be forced to override each configuration element" ?
(I think) Undefined variables do not affect at all the param values, since the the param-value of an undefined variable will be null, just as it would be without using the var expression. And for built-in types i use "default-value".
Also, pom-configuration works like it always does. Actually, pom values take precedence over any cmd-line vars.
I may have missed something here...correct me if i'm wrong or give me an example of what you mean. I have added var expressions to other plugins and if that is problematic, i would like to know, so as to correct them.
Regards,
Kostis
I did not mean to explain why it using properties in this manner could would... but why you want them.
I do not see the added value... this is m1's way of configuration and unless there is a good reason for it, I don't want to see this style come back in m2 plugins.
There are a few exceptions though... when you might need to alter the flow of the build in a transient way... say to skip tests or something. But I don't see that any of the configuration values here fit into that category.
If you just want to be able to translate from the command-line, I suggest you just use the CLI that comes with RetroTranslator:
BUT if you have a use case that does not fall into the full CLI that can show why these properties are helpful I'm willing to reconsider.
Well, i ussually configure from the cmd-line the 'verbose' and failOnWarning' .
I do this a lot, in particular on other plugins, since retrotranslator plugin once it has been setup correctly, it always behaves later. The usage pattern is like this:
For analogous params to 'verbose' and 'debug' i leave them unconfigured in the pom so they can be set from cmd line, see what the problem is, and fix it without touching the pom itself.
For the rest params, i add vars for consistency-no intention to drive back to maven-1...i hadn't thought of that danger
.
Here is an alphabetically list of the maven plugins that put expression's with variables into their mojo params. Those with an asterisk
, do not use the maven_plg_name.param_name pattern:
compiler, artifact
, assembly,
checkstyle, deploy, help
, install
, jar, javadoc
, jxr(*, not all),
My point is that i don't see any reason not to include vars into the mojo params. If you think this is an issue, then we should bring it to the maven and codehaus forums for discussion. I dont expect any disagreements, but who knows, there might be more sharing your views.
Best Regards
Kostis
(A job worth doing it...)
Here is the complete list:
compiler, artifact
, assembly
, checkstyle, deploy, help
, install
, jar, javadoc
, jxr(*, not all), one
, pmd, clean
, scm
, site
, source
, surefire
, surefire-report
, war
I remark that the "[maven].[plg_name].[param_name]" pattern is use by plugins that are not usually invoked from cmd-line (ie. the compiler plugin).
Now i see why Jason had such a hard time to incorporate the whole of patch.
The initial patch had a BUG (expressions were not sourrounded by quotes), and as a consequence, maven screamed when no value for verifyClasspath was specified in the pom, so i can now understand Jason's previous comments...
I deeply apologize for the typos. Kudos to you Jason, since you obviously rejected the patch after testing it . I'm sure that now you will find the new patch appropriate to apply.
The new patch 'retrotrans-plgn.doc-n-var-params.MOJO-459-2.patch' is against the current version, so it does not include the javadocs.
My Apologies
Kostis
There is nothing wrong with using expression to pick up components or predefined values. What I object to is setting expression to a property just so that one could set it with -Dmaven.someplugin.someproperty=blah on the command-line.
I certainly object to using ${maven.plugin.property} properties for this type of stuff. If we did add this for RT, then I'd say that ${retrotranslator.property} is better, but I still don't really like it ![]()
And... for items that are complex types (like List) not really sure what the expected behavior is for Maven to properly parse that.
It is also my understanding that for a configuration parameter of say 'verify', that you can execute the plugin and use -Dverify=true to change the value, assuming it was not configured in the pom... though I personally think that should be fixed in Maven to allow command-line to override pom values).
New patch BUG-free included. I'm now re-opening the issue since i suppose there is no dispute over the use of variables in the mojo's parameters, and the patch will be applied.
Hi Jason,
we were obviously writting at the same time...
Please i urge you to reconsider the whole issue, since my BUG made you draw horrible assumptions about the use of variables in parameters. When implemented right, it is transparent to the users of the plugin. On the contrary, my BUGgy version indeed forced users to define parameters inside the pom.
The use of variables is a common pattern among maven plugins, see the list of apache's standard maven plugins i gave above that are using it.
then I'd say that ${retrotranslator.property} is better, but I still don't really like it
I preffer to preffix variables with 'maven', since these are in fact system wide, and they may interfere with other components. See the excellent 'maven-compiler-plugin' among others.
I know it may sound as a bad joke, but the previous patch had also BUG, so i'm repatching...
Now i will never get that svn commit access ![]()
FYI... I just checked for all occurrences of ${maven. in the mojo and maven2/plugins sources and found only these:
- Mojo
- ideauidesigner-maven-plugin
- Mojo Sandbox
- cruisecontrol-maven-plugin
- hibernate3-maven-plugin
- Maven2 Plugins
- maven-ant-plugin (not using w/@expression)
- maven-compiler-plugin
- maven-jar-plugin
- maven-pmd-plugin
- maven-surefire-plugin
- maven-war-plugin
The usage in Maven2 plugins, mostly for compiler and surefire, but also in the others appears to be mostly carry over from the configuration that was used on Maven1.
So out of about 120 plugins (released and in the mojo sandbox)... 7 of them use ${maven.* in @expression to set configuration values.
If you like to discus this more on dev@mojo.codehaus.org I would be more than happy to do so... but for now, I'm not going to apply the changes to add ${maven. properties to @expression.
Please Jason,
now that i'm trying out the released version i would appreciate that <verbose> and <failonwarning> parameters had variable names, so as not to have to re-edit the pom.xml just for testing the plugin with verbose error messages but use the cmd-line -D switch instaed.
I would suggest the next plugin version to include variable names for (at least) those 2 parameters.
—
I would really like to know the opinion of the housmates about the 'params-with-vars' issue. So, i would appreciate if you could send an email to the mojo mailling list (an to apache mavane lists also) explaining the situation, and asking for general guidelines.
Greetings,
Kostis
mmmkay, I'm close to giving in... having started using this in a few other plugins... though I still only tend to add those for plugins intended to be invoked from he command line. But either way I think I will add them for you ![]()
Any progress on this issue (adding variable names to mojo params) ?
Do you need a new patch against the latest version ?
I've been stuck in the trenches of Geronimo build muck... no end in sight at the moment. If you have a patch against the latest code it would make it easier to get it in sooner.
Patch against recent revison is ready.
It just adds var-epressions to mojo params. Although it is very useful for the 'verify' and 'verbose' params, this patch defines expressions for the rest params, for homogenity.
Updated docs from retrotranslator ant task, added variables in the form used by compiler plugin, "$
{maven.[plg-name].[param_name]}".
Cannot document parameter "includes".