nfalco79 commented on a change in pull request #56:
URL: https://github.com/apache/maven-release/pull/56#discussion_r428658561
##########
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:
I had a look to the history of this file and also to the previous PR. I
have to agree with @rfscholte . The behavior to change the list to modifiable
list here is not clean and does something different than other methods that
also take a list.
Looking the deep the #50 it's better rework how the active profiles are
[added to the
ReleaseDescriptor](https://github.com/apache/maven-release/blob/b968279f72a12daea1836b795125a803bf7e8ccd/maven-release-manager/src/main/java/org/apache/maven/shared/release/DefaultReleaseManager.java#L317)
I do not understand why the
[performRequest.getReleaseDescriptorBuilder()](https://github.com/apache/maven-release/blob/b968279f72a12daea1836b795125a803bf7e8ccd/maven-release-manager/src/main/java/org/apache/maven/shared/release/DefaultReleaseManager.java#L304)
has a list of active profile and than after loadReleaseDescriptor
[this](https://github.com/apache/maven-release/blob/b968279f72a12daea1836b795125a803bf7e8ccd/maven-release-manager/src/main/java/org/apache/maven/shared/release/DefaultReleaseManager.java#L307)
some profile has gone so that is needed to be add them again.
----------------------------------------------------------------
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]
