Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)

Stuart McCulloch
Any string apart from "default" or the empty string will work here - I
happened to chose "core" because I viewed it as a core implementation.

Also a quick reminder that this is only needed when a component uses @Typed
and wants to allow overriding, it's not necessary in any other situation -
so in that sense "allowDefaultOverride" wouldn't be quite accurate. (See
the javadoc in the patch for more detail.)

PS. the reason DefaultModelProcessor needs to use @Typed is because it has
an "interesting" interface hierarchy where ModelProcessor extends
both ModelLocator and ModelReader, so it can act as both and delegate
accordingly - but then it also asks to have a ModelLocator and ModelReader
injected, which is where things get messy. If it had a cleaner hierarchy
(ie. it wasn't asking to inject something that it also implements) then it
wouldn't need @Typed and wouldn't then need the custom name.


On Fri, 18 Oct 2019 at 20:35, Robert Scholte <[hidden email]> wrote:

> Hi,
>
> the adjusted @Named annotation is on DefaultModelProcessor, not
> DefaultModelBuilder.
> They both implement the ModelBuilder interface, but the one that
> extensions like to overwrite is the implementation of DefaultModelBuilder.
> So I'd prefer to stick to "core" as proposed my Stuart.
>
> thanks for the confirmation that this works,
> Robert
>
> On Fri, 18 Oct 2019 21:10:18 +0200, Jason van Zyl <[hidden email]>
> wrote:
>
> > Hi,
> >
> > As noted in Slack I think it would be more clear if we used something
> > like
> >
> > @Named( “allowDefaultOverride” )
> >
> > vs
> >
> > @Named( “core” )
> >
> > As that expresses the intent and can be used anywhere it's allowed for
> a
> > client to override the default component.
> >
> > The tests in polyglot all pass with this change, and I’m able to run
> > polyglot example projects again, so I assume the Tycho POM-less are
> > happy again as well but hopefully someone can verify.
> >
> > Jason
> >
> >> On Oct 18, 2019, at 2:04 PM, Robert Scholte <[hidden email]>
> >> wrote:
> >>
> >> Hi,
> >>
> >> with the help from Stuart McCulloch we've been able to provide a patch
> >> for MNG-6765[1]
> >> Please review and test.
> >>
> >> thanks,
> >> Robert
> >>
> >> [1] https://issues.apache.org/jira/browse/MNG-67
> >> [2]
> >>
> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b874c
> >> [3]
> >>
> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a681b6
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: [hidden email]
> >> For additional commands, e-mail: [hidden email]
> >>
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Second MNG-6765 ([Regression] tycho pom-less builds fails with 3.6.2)

Tibor Digana
Are you talking about
@Named( “not-default” )
@Named( “coreAllowingOverride” ) or @Named( “coreExtensionPoint” )?

In Java EE (and these annotations are from Java EE application servers)
mean the name of the bean which is unique - it is not a group of beans.
Please notice that beans container is Map<String, Bean> simply speaking.
and therefore here it would mean :

"core-default" -> singleton instance (DefaultModelProcessor@1234567)

so this means a conflict because you cannot create:

"core-default" -> singleton instance (DefaultModelProcessor@1234567)
"core-default" -> singleton instance (DefaultResolver@1234567)
"core-default" -> singleton instance (AnotherBeanType@1234567)

logical would be to have Expression API from Java EE and:

"core-default-modelprocessor" -> singleton instance
(DefaultModelProcessor@1234567)
"core-default-resolver" -> singleton instance (DefaultResolver@1234567)
"core-default-xxx" -> singleton instance (AnotherBeanType@1234567)


On Sat, Oct 19, 2019 at 12:03 PM Hervé BOUTEMY <[hidden email]>
wrote:

> +1
> just added a comment on a typo
>
> for the name of the component, perhaps "core-default", but I won't
> complain
> about any choice: the javadoc is what was really needed
>
> notice: perhaps we have other component that should have the same
> improvement
> to permit overriding in the future
>
> Regards,
>
> Hervé
>
> Le vendredi 18 octobre 2019, 20:04:53 CEST Robert Scholte a écrit :
> > Hi,
> >
> > with the help from Stuart McCulloch we've been able to provide a patch
> for
> > MNG-6765[1]
> > Please review and test.
> >
> > thanks,
> > Robert
> >
> > [1] https://issues.apache.org/jira/browse/MNG-6765
> > [2]
> >
> https://github.com/apache/maven/commit/24e6c0ec0a87b6682513287a23c36db6996b8
> > 74c [3]
> >
> https://github.com/apache/maven/commit/53a70bc8543124569ee787725b2004bc92a68
> > 1b6
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>