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 <[email protected]> > > 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: [email protected] > >> For additional commands, e-mail: [email protected] > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
