Re: Sling IDE: Lifecycle Mapping Extension Point

[Konrad Windszus]

Just for completeness: This feature of m2e was added in the context of 
https://bugs.eclipse.org/bugs/show_bug.cgi?id=337010 
, so is actually pretty 
old, so we can safely rely on it.

> On 11 Oct 2016, at 09:04, Konrad Windszus  wrote:
> 
> This should not be necessary. Whenever there is an unknown packaging the 
> DefaultLifecycleMapping.java is used which derives from 
> AbstractCustomizableLifecycleMapping as well.
> There is even a test explicitly written for that in 
> https://github.com/tesla/m2e-core-tests/blob/2aea30eae086696da8ffce648c2eea010184b348/org.eclipse.m2e.tests/src/org/eclipse/m2e/tests/lifecycle/LifecycleMappingTest.java#L391
>  
> .
> So I go ahead and remove those parts from the m2e-ui plugin.
> 
>> On 11 Oct 2016, at 01:27, Georg Henzler > > wrote:
>> 
>> If I remember it correctly, it was needed to make the project configurator 
>> work at all. Note that ContentPackageLifecycleMapping is not just empty but 
>> extends AbstractCustomizableLifecycleMapping [1] and this code only becomes 
>> active for content packages because of the reference chain [2]. But maybe 
>> the situation has changed with current m2e versions, might be worth checking 
>> what impact removing the ContentPackageLifecycleMapping has.
>> 
>> Regards
>> Georg
>> 
>> [1]
>> https://github.com/jvanzyl/m2e-core/blob/master/org.eclipse.m2e.core/src/org/eclipse/m2e/core/project/configurator/AbstractCustomizableLifecycleMapping.java
>>  
>> 
>> 
>> [2]
>> packaging "content-package" -> id 
>> "org.apache.sling.ide.eclipse.m2e.contentPackageLifecycleMapping" -> 
>> ContentPackageLifecycleMapping that extends 
>> AbstractCustomizableLifecycleMapping
>> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/lifecycle-mapping-metadata.xml#L40
>> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/lifecycle-mapping-metadata.xml#L41
>> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/plugin.xml#L64
>> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/plugin.xml#L65
>> 
>> 
>> On 2016-10-10 15:05, Konrad Windszus wrote:
>>> In
>>> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/lifecycle-mapping-metadata.xml#L41
>>> 
>>> we have referenced the explicit lifecycle mapping id from
>>> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/plugin.xml#L62
>>> 
>>> which references the class
>>> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/src/org/apache/sling/ide/eclipse/m2e/internal/ContentPackageLifecycleMapping.java
>>> .
>>> The latter is empty.
>>> I am wondering why the lifecycle mapping extension point was ever
>>> necessary because we do nothing specific in
>>> ContentPackageLifecycleMapping.java
>>> 
>>> and m2e should be able to come up with a a default lifecycle id based
>>> on the configured plugin executions.
>>> If there is no good reason for that, I would rather get rid of the
>>> explicit lifecycle mapping id and only configure the plugin execution
>>> similar to the one for maven-bundle-plugin.
>>> @Georg Henzler: Because the original source code was contributed by
>>> you in https://issues.apache.org/jira/browse/SLING-3100
>>>  maybe you can share
>>> your thoughts.
>>> Thanks,
>>> Konrad
> 

Re: Sling IDE: Lifecycle Mapping Extension Point

[Konrad Windszus]

This should not be necessary. Whenever there is an unknown packaging the 
DefaultLifecycleMapping.java is used which derives from 
AbstractCustomizableLifecycleMapping as well.
There is even a test explicitly written for that in 
https://github.com/tesla/m2e-core-tests/blob/2aea30eae086696da8ffce648c2eea010184b348/org.eclipse.m2e.tests/src/org/eclipse/m2e/tests/lifecycle/LifecycleMappingTest.java#L391
 
.
So I go ahead and remove those parts from the m2e-ui plugin.

> On 11 Oct 2016, at 01:27, Georg Henzler  wrote:
> 
> If I remember it correctly, it was needed to make the project configurator 
> work at all. Note that ContentPackageLifecycleMapping is not just empty but 
> extends AbstractCustomizableLifecycleMapping [1] and this code only becomes 
> active for content packages because of the reference chain [2]. But maybe the 
> situation has changed with current m2e versions, might be worth checking what 
> impact removing the ContentPackageLifecycleMapping has.
> 
> Regards
> Georg
> 
> [1]
> https://github.com/jvanzyl/m2e-core/blob/master/org.eclipse.m2e.core/src/org/eclipse/m2e/core/project/configurator/AbstractCustomizableLifecycleMapping.java
> 
> [2]
> packaging "content-package" -> id 
> "org.apache.sling.ide.eclipse.m2e.contentPackageLifecycleMapping" -> 
> ContentPackageLifecycleMapping that extends 
> AbstractCustomizableLifecycleMapping
> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/lifecycle-mapping-metadata.xml#L40
> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/lifecycle-mapping-metadata.xml#L41
> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/plugin.xml#L64
> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/plugin.xml#L65
> 
> 
> On 2016-10-10 15:05, Konrad Windszus wrote:
>> In
>> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/lifecycle-mapping-metadata.xml#L41
>> 
>> we have referenced the explicit lifecycle mapping id from
>> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/plugin.xml#L62
>> 
>> which references the class
>> https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/src/org/apache/sling/ide/eclipse/m2e/internal/ContentPackageLifecycleMapping.java
>> .
>> The latter is empty.
>> I am wondering why the lifecycle mapping extension point was ever
>> necessary because we do nothing specific in
>> ContentPackageLifecycleMapping.java
>> 
>> and m2e should be able to come up with a a default lifecycle id based
>> on the configured plugin executions.
>> If there is no good reason for that, I would rather get rid of the
>> explicit lifecycle mapping id and only configure the plugin execution
>> similar to the one for maven-bundle-plugin.
>> @Georg Henzler: Because the original source code was contributed by
>> you in https://issues.apache.org/jira/browse/SLING-3100
>>  maybe you can share
>> your thoughts.
>> Thanks,
>> Konrad

Re: Sling IDE: Lifecycle Mapping Extension Point

[Georg Henzler]

If I remember it correctly, it was needed to make the project 
configurator work at all. Note that ContentPackageLifecycleMapping is 
not just empty but extends AbstractCustomizableLifecycleMapping [1] and 
this code only becomes active for content packages because of the 
reference chain [2]. But maybe the situation has changed with current 
m2e versions, might be worth checking what impact removing the 
ContentPackageLifecycleMapping has.


Regards
Georg

[1]
https://github.com/jvanzyl/m2e-core/blob/master/org.eclipse.m2e.core/src/org/eclipse/m2e/core/project/configurator/AbstractCustomizableLifecycleMapping.java

[2]
packaging "content-package" -> id 
"org.apache.sling.ide.eclipse.m2e.contentPackageLifecycleMapping" -> 
ContentPackageLifecycleMapping that extends 
AbstractCustomizableLifecycleMapping

https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/lifecycle-mapping-metadata.xml#L40
https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/lifecycle-mapping-metadata.xml#L41
https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/plugin.xml#L64
https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/plugin.xml#L65


On 2016-10-10 15:05, Konrad Windszus wrote:

In
https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/lifecycle-mapping-metadata.xml#L41

we have referenced the explicit lifecycle mapping id from
https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/plugin.xml#L62

which references the class
https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/src/org/apache/sling/ide/eclipse/m2e/internal/ContentPackageLifecycleMapping.java
.
The latter is empty.
I am wondering why the lifecycle mapping extension point was ever
necessary because we do nothing specific in
ContentPackageLifecycleMapping.java

and m2e should be able to come up with a a default lifecycle id based
on the configured plugin executions.

If there is no good reason for that, I would rather get rid of the
explicit lifecycle mapping id and only configure the plugin execution
similar to the one for maven-bundle-plugin.

@Georg Henzler: Because the original source code was contributed by
you in https://issues.apache.org/jira/browse/SLING-3100
 maybe you can share
your thoughts.

Thanks,
Konrad

Sling IDE: Lifecycle Mapping Extension Point

[Konrad Windszus]

In 
https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/lifecycle-mapping-metadata.xml#L41
 

 we have referenced the explicit lifecycle mapping id from 
https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/plugin.xml#L62
 

 which references the class 
https://github.com/apache/sling/blob/trunk/tooling/ide/eclipse-m2e-ui/src/org/apache/sling/ide/eclipse/m2e/internal/ContentPackageLifecycleMapping.java
 
.
 The latter is empty.
I am wondering why the lifecycle mapping extension point was ever necessary 
because we do nothing specific in ContentPackageLifecycleMapping.java 

 and m2e should be able to come up with a a default lifecycle id based on the 
configured plugin executions.

If there is no good reason for that, I would rather get rid of the explicit 
lifecycle mapping id and only configure the plugin execution similar to the one 
for maven-bundle-plugin.

@Georg Henzler: Because the original source code was contributed by you in 
https://issues.apache.org/jira/browse/SLING-3100 
 maybe you can share your 
thoughts.

Thanks,
Konrad