nielsbasjes commented on code in PR #104:
URL: https://github.com/apache/maven-release/pull/104#discussion_r927512544
##########
maven-release-manager/src/main/java/org/apache/maven/shared/release/config/PropertiesReleaseDescriptorStore.java:
##########
@@ -271,7 +271,7 @@ public void write( BuilderReleaseDescriptor config, File
file )
}
if ( config.getProjectVersionPolicyConfig() != null )
{
- properties.setProperty( "projectVersionPolicyConfig",
config.getProjectVersionPolicyConfig() );
+ properties.setProperty( "projectVersionPolicyConfig",
config.getProjectVersionPolicyConfig().toString() );
Review Comment:
I did some more check and currently I think is actually correct code.
What I have now:
- By setting the type to DOM the generated code makes the class of this
thing an `Object` in Java (i.e. all getters and setters). I checks and the
actuall class is an instance of
org.codehaus.plexus.configuration.xml.XmlPlexusConfiguration. So the toString()
recreates the XML fragment.
- Some of the code can only handle a `String` (like for example the parts
that reads and writes the release.properties file).
So the code path now is roughly:
- The `ProjectVersionPolicyConfig` part in the pom.xml is parsed into a
generic XmlPlexusConfiguration by Maven.
- This XmlPlexusConfiguration is converted into a `String` so it can be
stored and processed by all intermediate components (some of which can only
handle String).
- The actual VersionPolicy has it's own parser to make sense of the provided
String.
I did two tests to verify this:
1) In the integration test I have put this in the pom.xml
```
<projectVersionPolicyConfig>
<versionTag>^v([0-9]+(?:\.[0-9]+(?:\.[0-9]+)?)?)$</versionTag>
<majorRules>
<majorRule>^[a-zA-Z]+!(?:\([a-zA-Z0-9_-]+\))?: .*$</majorRule>
<majorRule>^BREAKING CHANGE:.*$</majorRule>
</majorRules>
<minorRules>
<minorRule>^feat(?:\([a-zA-Z0-9_-]+\))?: .*$</minorRule>
</minorRules>
</projectVersionPolicyConfig>
```
I see this in the release.properties:
```
projectVersionPolicyConfig=<projectVersionPolicyConfig><versionTag>^v([0-9]+(?\:\\.[0-9]+(?\:\\.[0-9]+)?)?)$</versionTag>\n<majorRules><majorRule>^[a-zA-Z]+\!(?\:\\([a-zA-Z0-9_-]+\\))?\:
.*$</majorRule>\n<majorRule>^BREAKING
CHANGE\:.*$</majorRule>\n</majorRules>\n<minorRules><minorRule>^feat(?\:\\([a-zA-Z0-9_-]+\\))?\:
.*$</minorRule>\n</minorRules>\n</projectVersionPolicyConfig>\n
```
2) To see if `any` config format is usable I put this in the pom.xml
```
<projectVersionPolicyConfig>
<![CDATA[
config:
- something:
foo:
- 'foo foo'
bar:
- 'bar bar'
]]>
</projectVersionPolicyConfig>
```
I see this in the release.properties
```
projectVersionPolicyConfig=<projectVersionPolicyConfig>config\:\n -
something\:\n foo\:\n - 'foo foo'\n bar\:\n - 'bar
bar'</projectVersionPolicyConfig>\n
```
In both these checks the `toString` made sure the right config is stored and
also by `forcing` the VersionPolicy to have its own parser it works as I would
like it to work.
This way it is up to the VersionPolicy to actually parse the config and
verify what it is getting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
