maven-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Hervé BOUTEMY <herve.bout...@free.fr>
Subject Re: [MNG-6164] Collections inconsistently immutable
Date Sun, 28 May 2017 16:36:23 GMT
I started to review each modification in depth and write a summary, but I 
stopped since I know nobody will read it because of TLDR;

I preferred rewrite the issue description as:
"All those methods should return a collection with consistent "mutability": 
either mutable, either immutable.

Given empty immutable collections do not cause harm until now, switching 
consistently to immutable collections is more conservative and should not be 
risky"

I think it would be useful to add the info on immutable collection in API 
javadoc, since some classes are not consistent on every returned collection 
(for example DefaultArtifact, where getDependencyTrail() and 
getAvailableVersions() stay mutable)


From what I saw, the change does not really give the documented API 
consistency that one could expect, but should not make real harm (I hope).
And I buy your "Changing the returned list [...] should be an easy fix by 
plugin maintainers"

I won't second this change but I won't block it

Regards,

Hervé

Le dimanche 28 mai 2017, 17:01:21 CEST Robert Scholte a écrit :
> I think we should do this step, either now or with a next major release.
> 
> And it's about the *returned* Lists which now become unmodifiable. As far
> as I can see all these changes have a setter which should be used to
> specify a new list.
> These changes should protect developers which use the returned list and
> manipulate it for own business logic (e.g. remove unwanted entries). But
> this should not change the original state, unless they called the matching
> setter.
> Changing the returned list should be considered a bad practice and should
> be an easy fix by plugin maintainers.
> 
> Robert
> 
> On Sun, 28 May 2017 13:01:41 +0200, Hervé BOUTEMY <herve.boutemy@free.fr>
> 
> wrote:
> > I'm not confident on this one.
> > 
> > I understand the inconsistency issue, that may lead to edge-case issues
> > in
> > plugins or extensions (that unexpectedly get an immutable empty list
> > when they
> > usually get a modifiable non-empty list)
> > 
> > Is it the right solution to change every non-empty list to immutable?
> > Why wouldn't be the solution to make every returned empty list mutable?
> > Or a mix: on some cases, immutable is a good feature, but on others
> > consistent
> > mutable would be the good choice?
> > 
> > Do you know how many plugins or extensions this update will break?
> > I know that consistent breaks are better than inconsistent breaks (on
> > empty
> > lists), but is it the right time to take such risk?
> > 
> > I know that core ITs pass: that does not mean that external plugins won't
> > break (thousands in Central only, I don't know how many corporate plugins
> > exist)
> > 
> > 
> > How many cases are changed? In which areas of Maven API?
> > That is the information I need to vote for this change, knowing the
> > effective
> > impact it will have
> > 
> > Regards,
> > 
> > Hervé
> > 
> > https://issues.apache.org/jira/browse/MNG-6164
> > 
> > Le dimanche 28 mai 2017, 12:01:20 CEST Michael Osipov a écrit :
> >> Am 2017-05-25 um 21:12 schrieb Michael Osipov:
> >> > Who seconds MNG-6164 for 3.5.1?
> >> > 
> >> > This is a non-functional consistency fix. All ITs pass.
> >> 
> >> Anyone?
> >> 
> >> 
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> >> For additional commands, e-mail: dev-help@maven.apache.org
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > For additional commands, e-mail: dev-help@maven.apache.org
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Mime
View raw message