nfalco79 commented on a change in pull request #56:
URL: https://github.com/apache/maven-release/pull/56#discussion_r429150187



##########
File path: 
maven-release-manager/src/main/java/org/apache/maven/shared/release/config/ReleaseDescriptorBuilder.java
##########
@@ -58,7 +59,9 @@ public ReleaseDescriptorBuilder addCheckModificationExclude( 
String string )
 
     public ReleaseDescriptorBuilder setActivateProfiles( List<String> profiles 
)
     {
-        releaseDescriptor.setActivateProfiles( profiles );
+        List<String> copy = new ArrayList<>();
+        copy.addAll( profiles );

Review comment:
       > 
   > 
   > In the `DefaultReleaseManager` you can already see some anonymous classes 
that are used to control such logic.
   
   I see. Maybe could be the case implement an inner class 
MergeReleaseDescriptorBuilder and replace all the instancies unless in some 
cases you really want to merge only a specific properties. Anyway this way it's 
quite tricky, I say tricky because it's applied more times so the idea of a 
ReleaseDescriptorBuilder unmutable seems to be not so strong.
   ```
   ReleaseDescriptor releaseDescriptor = loadReleaseDescriptor(
       new ReleaseDescriptorBuilder() {
           public void setActivateProfiles( List<String> profiles ) {
               builder = performRequest.getReleaseDescriptorBuilder()
               // merge with actual
               
builder.setActivateProfiles(builder.build().getActivateProfiles() + profiles);
           }
       }, performRequest.getReleaseManagerListener() );
   ```




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to