aleksandr-m / gitflow-maven-plugin

The Git-Flow Maven Plugin supports various Git workflows, including GitFlow and GitHub Flow. This plugin runs Git and Maven commands from the command line.
https://aleksandr-m.github.io/gitflow-maven-plugin/
Apache License 2.0
490 stars 182 forks source link

Implementing command-line overrides for configuration #285

Closed kutsal closed 2 years ago

kutsal commented 3 years ago

Fix for #284 where CLI parameters in the form of -DgitFlowConfig.developmentBranch=foobar or -DcommitMessages.hotfixVersionUpdateMessage=hello override pom-specified plugin configuration.

aleksandr-m commented 2 years ago

@kutsal I've played a bit with this solution. It doesn't set value if there is any gitFlowConfig in pom, even if there is no specific property of gitFlowConfig in it. Any ideas how this can be improved?

Where did you find information about using set method in maven? Any links?

kutsal commented 2 years ago

@aleksandr-m that is correct, because like you mentioned in your response to #284, entries in pom take precedence, always, so specifying gitFlowConfig in the pom essentially eliminates the call to the set method.

As for where I found this; Maven uses Plexus as for IoC, and that uses Sisu which handles classpath scanning, binding, wiring, setting values, etc. I was trying to learn how these (gitFlowConfig-like) beans get their values wired in by Maven.

The key, I discovered, was the setDefault method of CompositeBeanHelper in Sisu, specifically this line. The @Parameter annotation on the Maven configuration POJO that maps the defaultValue to an expression triggers this particular "feature".

While it feels somewhat hacky to me, it works fine for this purpose without relying on System properties or other constructor or set-method based tomfoolery.

aleksandr-m commented 2 years ago

@kutsal Thank you for details. I've checked again the #263 PR and besides using System properties it seems pretty good solution.

I'll see if userProperties from maven session can be somehow used instead of System, and I probably can implement/test this in a few next days.

What do you think?

kutsal commented 2 years ago

Works for me. We've been using my implementation (this PR) for a while, and have had good results with it. Once the plugin supports the CLI-based configuration, we can switch to that (official) version.

aleksandr-m commented 2 years ago

Seems there is no clean way to use mavenSession.getUserProperties() in GitFlowConfig, so let's see how System.getProperty will manage. Implemented in https://github.com/aleksandr-m/gitflow-maven-plugin/commit/f1adadc44b22895035bdb8cd4d49c748582a515b.

aleksandr-m commented 2 years ago

@kutsal 1.17.0 is released.