[GitHub] [sling-org-apache-sling-models-impl] paul-bjorkstrand commented on a change in pull request #14: SLING-8655 - Updated dependency on API, provided Externalized Path In…

2019-08-23 Thread GitBox
paul-bjorkstrand commented on a change in pull request #14: SLING-8655 - 
Updated dependency on API, provided Externalized Path In…
URL: 
https://github.com/apache/sling-org-apache-sling-models-impl/pull/14#discussion_r317337536
 
 

 ##
 File path: 
src/main/java/org/apache/sling/models/impl/injectors/ExternalizedPathInjector.java
 ##
 @@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.models.impl.injectors;
+
+import org.apache.commons.lang3.ObjectUtils;
+import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.ValueMap;
+import org.apache.sling.models.annotations.ExternalizePath;
+import 
org.apache.sling.models.annotations.injectorspecific.ExternalizedPathProvider;
+import org.apache.sling.models.spi.DisposalCallbackRegistry;
+import org.apache.sling.models.spi.Injector;
+import org.jetbrains.annotations.NotNull;
+import org.osgi.framework.Constants;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Reference;
+import org.osgi.service.component.annotations.ReferenceCardinality;
+import org.osgi.service.component.annotations.ReferencePolicy;
+
+import java.lang.reflect.AnnotatedElement;
+import java.lang.reflect.Type;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+@Component(
+property=Constants.SERVICE_RANKING+":Integer=1000",
+service={
+Injector.class
+}
+)
+public class ExternalizedPathInjector
+extends AbstractInjector
+implements Injector
+{
+List providerList = new ArrayList<>();
+
+@Reference(policy = ReferencePolicy.DYNAMIC, cardinality = 
ReferenceCardinality.OPTIONAL)
+void bindExternalizedPathProvider(ExternalizedPathProvider provider) {
+providerList.add(provider);
+// The providers are sorted so that the one with the highest priority 
is the first entry
+Collections.sort(
+providerList,
+
Comparator.comparingInt(ExternalizedPathProvider::getPriority).reversed()
+);
+}
+
+void unbindExternalizedPathProvider(ExternalizedPathProvider provider) {
+providerList.remove(provider);
+}
+
+public ExternalizedPathInjector() {
+bindExternalizedPathProvider(new DefaultExternalizedPathProvider());
+}
+
+@Override
+public @NotNull String getName() {
+return "externalize-path";
+}
+
+@Override
+public Object getValue(@NotNull Object adaptable, String name, @NotNull 
Type type, @NotNull AnnotatedElement element,
+@NotNull DisposalCallbackRegistry callbackRegistry) {
+if (adaptable == ObjectUtils.NULL) {
+return null;
+}
+if (element.isAnnotationPresent(ExternalizePath.class)) {
+ValueMap properties = getValueMap(adaptable);
+if(properties != ObjectUtils.NULL) {
+String imagePath = properties.get(name, String.class);
+if(imagePath != null) {
+ExternalizedPathProvider provider = providerList.get(0);
+return provider.externalize(adaptable, imagePath);
+}
+}
+}
+return null;
+}
+
+/** Fallback Implementation of the Externalized Path Provider that uses 
the Resource Resolver's map function **/
+private class DefaultExternalizedPathProvider
 
 Review comment:
   This should be at least a `static` class, as it does not use its enclosing 
class at all. I would probably move this into its own class and make it a 
proper `@Component`.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [sling-org-apache-sling-models-impl] paul-bjorkstrand commented on a change in pull request #14: SLING-8655 - Updated dependency on API, provided Externalized Path In…

2019-08-23 Thread GitBox
paul-bjorkstrand commented on a change in pull request #14: SLING-8655 - 
Updated dependency on API, provided Externalized Path In…
URL: 
https://github.com/apache/sling-org-apache-sling-models-impl/pull/14#discussion_r317337946
 
 

 ##
 File path: 
src/main/java/org/apache/sling/models/impl/injectors/ExternalizedPathInjector.java
 ##
 @@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.models.impl.injectors;
+
+import org.apache.commons.lang3.ObjectUtils;
+import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.ValueMap;
+import org.apache.sling.models.annotations.ExternalizePath;
+import 
org.apache.sling.models.annotations.injectorspecific.ExternalizedPathProvider;
+import org.apache.sling.models.spi.DisposalCallbackRegistry;
+import org.apache.sling.models.spi.Injector;
+import org.jetbrains.annotations.NotNull;
+import org.osgi.framework.Constants;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Reference;
+import org.osgi.service.component.annotations.ReferenceCardinality;
+import org.osgi.service.component.annotations.ReferencePolicy;
+
+import java.lang.reflect.AnnotatedElement;
+import java.lang.reflect.Type;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+@Component(
+property=Constants.SERVICE_RANKING+":Integer=1000",
+service={
+Injector.class
+}
+)
+public class ExternalizedPathInjector
+extends AbstractInjector
+implements Injector
+{
+List providerList = new ArrayList<>();
+
+@Reference(policy = ReferencePolicy.DYNAMIC, cardinality = 
ReferenceCardinality.OPTIONAL)
+void bindExternalizedPathProvider(ExternalizedPathProvider provider) {
+providerList.add(provider);
+// The providers are sorted so that the one with the highest priority 
is the first entry
+Collections.sort(
+providerList,
+
Comparator.comparingInt(ExternalizedPathProvider::getPriority).reversed()
 
 Review comment:
   Instead of sorting based on a new field called `priority`, you should 
probably use `service.ranking` of the implementations.  Perhaps you want to 
look at how ModelAdapterFactory uses [ranked services].
   
   [ranked services]: 
https://github.com/apache/sling-org-apache-sling-models-impl/blob/398a382724154eb6bd80bd9d4142f100fee2532c/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L217


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [sling-org-apache-sling-models-impl] paul-bjorkstrand commented on a change in pull request #14: SLING-8655 - Updated dependency on API, provided Externalized Path In…

2019-08-23 Thread GitBox
paul-bjorkstrand commented on a change in pull request #14: SLING-8655 - 
Updated dependency on API, provided Externalized Path In…
URL: 
https://github.com/apache/sling-org-apache-sling-models-impl/pull/14#discussion_r317338515
 
 

 ##
 File path: 
src/test/java/org/apache/sling/models/impl/injectors/ExternalizedPathInjectorTest.java
 ##
 @@ -0,0 +1,128 @@
+package org.apache.sling.models.impl.injectors;
+
+import org.apache.sling.api.resource.Resource;
+import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.ValueMap;
+import org.apache.sling.models.annotations.ExternalizePath;
+import 
org.apache.sling.models.annotations.injectorspecific.ExternalizedPathProvider;
+import org.apache.sling.models.spi.DisposalCallbackRegistry;
+import org.jetbrains.annotations.NotNull;
+import org.junit.Test;
+
+import java.lang.reflect.AnnotatedElement;
+import java.lang.reflect.Type;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class ExternalizedPathInjectorTest {
+
+@Test
+public void testNoResolveInjection() {
+String imagePath = "/content/test/image/test-image.jpg";
+
+ExternalizedPathInjector injector = new ExternalizedPathInjector();
+Resource adaptable = mock(Resource.class);
+ValueMap valueMap = mock(ValueMap.class);
+when(adaptable.adaptTo(eq(ValueMap.class))).thenReturn(valueMap);
+String name = "imagePath";
+when(valueMap.get(eq(name), eq(String.class))).thenReturn(imagePath);
+Type type = String.class;
+AnnotatedElement element = mock(AnnotatedElement.class);
+
when(element.isAnnotationPresent(eq(ExternalizePath.class))).thenReturn(true);
+DisposalCallbackRegistry callbackRegistry = 
mock(DisposalCallbackRegistry.class);
+ResourceResolver resourceResolver = mock(ResourceResolver.class);
+when(adaptable.getResourceResolver()).thenReturn(resourceResolver);
+when(resourceResolver.map(imagePath)).thenReturn(imagePath);
+
+Object value = injector.getValue(adaptable, name, type, element, 
callbackRegistry);
+assertEquals("No Mapping was expected", imagePath, value);
+}
+
+@Test
+public void testResolveInjection() {
+String imagePath = "/content/test/image/test-image.jpg";
+String mappedImagePath = "/image/test-image.jpg";
+
+ExternalizedPathInjector injector = new ExternalizedPathInjector();
+Resource adaptable = mock(Resource.class);
+ValueMap valueMap = mock(ValueMap.class);
+when(adaptable.adaptTo(eq(ValueMap.class))).thenReturn(valueMap);
+String name = "imagePath";
+when(valueMap.get(eq(name), eq(String.class))).thenReturn(imagePath);
+Type type = String.class;
+AnnotatedElement element = mock(AnnotatedElement.class);
+
when(element.isAnnotationPresent(eq(ExternalizePath.class))).thenReturn(true);
+DisposalCallbackRegistry callbackRegistry = 
mock(DisposalCallbackRegistry.class);
+ResourceResolver resourceResolver = mock(ResourceResolver.class);
+when(adaptable.getResourceResolver()).thenReturn(resourceResolver);
+when(resourceResolver.map(imagePath)).thenReturn(mappedImagePath);
+
+Object value = injector.getValue(adaptable, name, type, element, 
callbackRegistry);
+assertEquals("Mapping was expected", mappedImagePath, value);
+}
+
+@Test
+public void testCustomProviderInjection() {
+String imagePath = "/content/test/image/test-image.jpg";
+String from = "/content/test/image/";
+String to1 = "/image1/";
+String to2 = "/image2/";
+String to3 = "/image3/";
+String mappedImagePath = "/image/test-image.jpg";
+String mappedImagePath1 = "/image1/test-image.jpg";
+String mappedImagePath2 = "/image2/test-image.jpg";
+String mappedImagePath3 = "/image3/test-image.jpg";
+
+ExternalizedPathInjector injector = new ExternalizedPathInjector();
+Resource adaptable = mock(Resource.class);
+ValueMap valueMap = mock(ValueMap.class);
+when(adaptable.adaptTo(eq(ValueMap.class))).thenReturn(valueMap);
+String name = "imagePath";
+when(valueMap.get(eq(name), eq(String.class))).thenReturn(imagePath);
+Type type = String.class;
+AnnotatedElement element = mock(AnnotatedElement.class);
+
when(element.isAnnotationPresent(eq(ExternalizePath.class))).thenReturn(true);
+DisposalCallbackRegistry callbackRegistry = 
mock(DisposalCallbackRegistry.class);
+ResourceResolver resourceResolver = mock(ResourceResolver.class);
+when(adaptable.getResourceResolver()).thenReturn(resourceResolver);
+when(resourceResolver.map(imagePath)).thenReturn(mappedImagePath);
+
+

[GitHub] [sling-org-apache-sling-models-impl] paul-bjorkstrand commented on a change in pull request #14: SLING-8655 - Updated dependency on API, provided Externalized Path In…

2019-08-23 Thread GitBox
paul-bjorkstrand commented on a change in pull request #14: SLING-8655 - 
Updated dependency on API, provided Externalized Path In…
URL: 
https://github.com/apache/sling-org-apache-sling-models-impl/pull/14#discussion_r317338005
 
 

 ##
 File path: 
src/main/java/org/apache/sling/models/impl/injectors/ExternalizedPathInjector.java
 ##
 @@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sling.models.impl.injectors;
+
+import org.apache.commons.lang3.ObjectUtils;
+import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.ValueMap;
+import org.apache.sling.models.annotations.ExternalizePath;
+import 
org.apache.sling.models.annotations.injectorspecific.ExternalizedPathProvider;
+import org.apache.sling.models.spi.DisposalCallbackRegistry;
+import org.apache.sling.models.spi.Injector;
+import org.jetbrains.annotations.NotNull;
+import org.osgi.framework.Constants;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Reference;
+import org.osgi.service.component.annotations.ReferenceCardinality;
+import org.osgi.service.component.annotations.ReferencePolicy;
+
+import java.lang.reflect.AnnotatedElement;
+import java.lang.reflect.Type;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+@Component(
+property=Constants.SERVICE_RANKING+":Integer=1000",
+service={
+Injector.class
+}
+)
+public class ExternalizedPathInjector
+extends AbstractInjector
+implements Injector
+{
+List providerList = new ArrayList<>();
+
+@Reference(policy = ReferencePolicy.DYNAMIC, cardinality = 
ReferenceCardinality.OPTIONAL)
+void bindExternalizedPathProvider(ExternalizedPathProvider provider) {
+providerList.add(provider);
+// The providers are sorted so that the one with the highest priority 
is the first entry
+Collections.sort(
+providerList,
+
Comparator.comparingInt(ExternalizedPathProvider::getPriority).reversed()
+);
+}
+
+void unbindExternalizedPathProvider(ExternalizedPathProvider provider) {
+providerList.remove(provider);
+}
+
+public ExternalizedPathInjector() {
+bindExternalizedPathProvider(new DefaultExternalizedPathProvider());
+}
+
+@Override
+public @NotNull String getName() {
+return "externalize-path";
+}
+
+@Override
+public Object getValue(@NotNull Object adaptable, String name, @NotNull 
Type type, @NotNull AnnotatedElement element,
+@NotNull DisposalCallbackRegistry callbackRegistry) {
+if (adaptable == ObjectUtils.NULL) {
+return null;
+}
+if (element.isAnnotationPresent(ExternalizePath.class)) {
+ValueMap properties = getValueMap(adaptable);
+if(properties != ObjectUtils.NULL) {
 
 Review comment:
   nitpicky formatting on this `if` and the `if` on line 85 as well.


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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Commented] (SLING-8655) Add an Annotation to Sling Model to mark a property to be Externalized

2019-08-23 Thread Andreas Schaefer (Jira)


[ 
https://issues.apache.org/jira/browse/SLING-8655?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16914561#comment-16914561
 ] 

Andreas Schaefer commented on SLING-8655:
-

Create PRs for this:

Sling Models API: 
https://github.com/apache/sling-org-apache-sling-models-api/pull/2

Sling Models Impl: 
https://github.com/apache/sling-org-apache-sling-models-impl/pull/14

> Add an Annotation to Sling Model to mark a property to be Externalized
> --
>
> Key: SLING-8655
> URL: https://issues.apache.org/jira/browse/SLING-8655
> Project: Sling
>  Issue Type: New Feature
>  Components: Sling Models
>Affects Versions: Sling Models API 1.3.8
>Reporter: Andreas Schaefer
>Assignee: Andreas Schaefer
>Priority: Major
> Fix For: Sling Models API 1.3.10
>
>  Time Spent: 20m
>  Remaining Estimate: 0h
>
> For Peregrine CMS we use Sling Models to obtain data in JSon format to be 
> rendered on the client side. This means that the returned content is not 
> externalized aka paths are not mapped to the external view.
> Sling Model should have an Annotation (@ExternalizedPath) that marks a 
> property to be externalized when loaded.
> In order to be flexible the Externalized Path Injector should be pluggable so 
> that customers can add their custom Externalized Path Providers if they 
> choose to do so. By default there is a provider that uses the Resource 
> Resolver's map() function.



--
This message was sent by Atlassian Jira
(v8.3.2#803003)


[GitHub] [sling-org-apache-sling-models-impl] schaefa opened a new pull request #14: SLING-8655 - Updated dependency on API, provided Externalized Path In…

2019-08-23 Thread GitBox
schaefa opened a new pull request #14: SLING-8655 - Updated dependency on API, 
provided Externalized Path In…
URL: https://github.com/apache/sling-org-apache-sling-models-impl/pull/14
 
 
   …jector (with default Provider) and an unit test
   
   This PR depends on PR of the Sling Models API: 
https://github.com/apache/sling-org-apache-sling-models-api/pull/2


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [sling-org-apache-sling-models-api] schaefa opened a new pull request #2: SLING-8655 - Added the Externalized Path Annotation and Provider inte…

2019-08-23 Thread GitBox
schaefa opened a new pull request #2: SLING-8655 - Added the Externalized Path 
Annotation and Provider inte…
URL: https://github.com/apache/sling-org-apache-sling-models-api/pull/2
 
 
   …rface


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:
us...@infra.apache.org


With regards,
Apache Git Services


[jira] [Created] (SLING-8655) Add an Annotation to Sling Model to mark a property to be Externalized

2019-08-23 Thread Andreas Schaefer (Jira)
Andreas Schaefer created SLING-8655:
---

 Summary: Add an Annotation to Sling Model to mark a property to be 
Externalized
 Key: SLING-8655
 URL: https://issues.apache.org/jira/browse/SLING-8655
 Project: Sling
  Issue Type: New Feature
  Components: Sling Models
Affects Versions: Sling Models API 1.3.8
Reporter: Andreas Schaefer
Assignee: Andreas Schaefer
 Fix For: Sling Models API 1.3.10


For Peregrine CMS we use Sling Models to obtain data in JSon format to be 
rendered on the client side. This means that the returned content is not 
externalized aka paths are not mapped to the external view.

Sling Model should have an Annotation (@ExternalizedPath) that marks a property 
to be externalized when loaded.

In order to be flexible the Externalized Path Injector should be pluggable so 
that customers can add their custom Externalized Path Providers if they choose 
to do so. By default there is a provider that uses the Resource Resolver's 
map() function.



--
This message was sent by Atlassian Jira
(v8.3.2#803003)


[jira] [Created] (SLING-8654) commons html - html5 parser doesn't handle doctype correctly

2019-08-23 Thread Jason E Bailey (Jira)
Jason E Bailey created SLING-8654:
-

 Summary: commons  html - html5 parser doesn't handle doctype 
correctly
 Key: SLING-8654
 URL: https://issues.apache.org/jira/browse/SLING-8654
 Project: Sling
  Issue Type: Bug
Reporter: Jason E Bailey
Assignee: Jason E Bailey


doctype is not handled correctly by the parser due to a regression when the 
parser was refactored, it's handling the doctype like a start tag instead of 
declaration.

 

 



--
This message was sent by Atlassian Jira
(v8.3.2#803003)


Re: [VOTE] Release Apache Sling Health Check Core 2.0.0, Sling Health Check Web Console 2.0.0

2019-08-23 Thread Andreas Schaefer
+1 (non-binding)

- Andy

> On Aug 23, 2019, at 2:35 AM, Radu Cotescu  wrote:
> 
> +1
> 
>> On 22 Aug 2019, at 19:34, Georg Henzler  wrote:
>> 
>> Please vote to approve this release:
>> 
>> [ ] +1 Approve the release
>> [ ]  0 Don't care
>> [ ] -1 Don't release, because ...
>> 
>> This majority vote is open for at least 72 hours.
> 



Re: Release Validator Docker Image

2019-08-23 Thread Daniel Klco
Radu,

In concept I agree, no good reason to have multiple tools to do similar
things. I wonder if we just fork off the release validation process into a
separate script vs the Java-based processes. I'm just not convinced of the
value re-implementing in Java brings vs a few simple bash commands. Let me
poke around and see if I can come up with something.

Thanks,
Dan

On Fri, Aug 23, 2019 at 6:31 AM Radu Cotescu  wrote:

> Hello,
>
> I’m +1 for any tool that reduces the number of manual steps I have to do
> when performing, validating or promoting a release.
>
> I have one question though: couldn’t we push these features into the
> Apache Sling Committer CLI tool [0]? That tool is already based on a
> minimalistic Docker image with an embedded jlink-customised JRE. Sensible
> information is provided from the outside anyways (think Apache user and
> password, JIRA user and password), so if we’re thinking about the keys file
> we could do something similar.
>
> The only reason for asking such a question is that it would be nice to
> consolidate all CLI checks into one tool.
>
> Thanks,
> Radu
>
> [0] - https://github.com/apache/sling-org-apache-sling-committer-cli <
> https://github.com/apache/sling-org-apache-sling-committer-cli>


[GitHub] [sling-org-apache-sling-feature-cpconverter] simonetripodi merged pull request #19: SLING-8649 - Missing dependencies when installing computed content-packages

2019-08-23 Thread GitBox
simonetripodi merged pull request #19: SLING-8649 - Missing dependencies when 
installing computed content-packages
URL: 
https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/19
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [sling-org-apache-sling-feature-cpconverter] simonetripodi opened a new pull request #19: SLING-8649 - Missing dependencies when installing computed content-packages

2019-08-23 Thread GitBox
simonetripodi opened a new pull request #19: SLING-8649 - Missing dependencies 
when installing computed content-packages
URL: 
https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/19
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


Re: Release Validator Docker Image

2019-08-23 Thread Radu Cotescu
Hello,

I’m +1 for any tool that reduces the number of manual steps I have to do when 
performing, validating or promoting a release.

I have one question though: couldn’t we push these features into the Apache 
Sling Committer CLI tool [0]? That tool is already based on a minimalistic 
Docker image with an embedded jlink-customised JRE. Sensible information is 
provided from the outside anyways (think Apache user and password, JIRA user 
and password), so if we’re thinking about the keys file we could do something 
similar.

The only reason for asking such a question is that it would be nice to 
consolidate all CLI checks into one tool.

Thanks,
Radu

[0] - https://github.com/apache/sling-org-apache-sling-committer-cli 


[GitHub] [sling-org-apache-sling-pipes] npeltier merged pull request #11: SLING-8648 Move pipe to incorporate ordering of the nodes .

2019-08-23 Thread GitBox
npeltier merged pull request #11: SLING-8648 Move pipe to incorporate ordering 
of the nodes .
URL: https://github.com/apache/sling-org-apache-sling-pipes/pull/11
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [sling-org-apache-sling-pipes] npeltier commented on a change in pull request #11: SLING-8648 Move pipe to incorporate ordering of the nodes .

2019-08-23 Thread GitBox
npeltier commented on a change in pull request #11: SLING-8648 Move pipe to 
incorporate ordering of the nodes .
URL: 
https://github.com/apache/sling-org-apache-sling-pipes/pull/11#discussion_r317057647
 
 

 ##
 File path: src/main/java/org/apache/sling/pipes/internal/MovePipe.java
 ##
 @@ -116,4 +126,25 @@ public boolean modifiesContent() {
 }
 return output;
 }
+
+private Iterator reorder(Resource resource, String targetPath, 
Session session) throws Exception {
+logger.debug("ordering {} before {}", resource.getPath(), targetPath);
 
 Review comment:
   should be info


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [sling-org-apache-sling-pipes] npeltier commented on a change in pull request #11: SLING-8648 Move pipe to incorporate ordering of the nodes .

2019-08-23 Thread GitBox
npeltier commented on a change in pull request #11: SLING-8648 Move pipe to 
incorporate ordering of the nodes .
URL: 
https://github.com/apache/sling-org-apache-sling-pipes/pull/11#discussion_r317057501
 
 

 ##
 File path: src/main/java/org/apache/sling/pipes/internal/MovePipe.java
 ##
 @@ -62,24 +67,29 @@ public boolean modifiesContent() {
 String targetPath = getExpr();
 try {
 Session session = resolver.adaptTo(Session.class);
-if (session.itemExists(targetPath)){
+if (orderBefore) {
+output = reorder(resource, targetPath, session);
+} else if (session.itemExists(targetPath)) {
 if (overwriteTarget && !isDryRun()) {
 logger.debug("overwriting {}", targetPath);
 
 Review comment:
   should be info


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [sling-org-apache-sling-pipes] npeltier commented on a change in pull request #11: SLING-8648 Move pipe to incorporate ordering of the nodes .

2019-08-23 Thread GitBox
npeltier commented on a change in pull request #11: SLING-8648 Move pipe to 
incorporate ordering of the nodes .
URL: 
https://github.com/apache/sling-org-apache-sling-pipes/pull/11#discussion_r317057021
 
 

 ##
 File path: src/main/java/org/apache/sling/pipes/internal/MovePipe.java
 ##
 @@ -62,24 +67,29 @@ public boolean modifiesContent() {
 String targetPath = getExpr();
 try {
 Session session = resolver.adaptTo(Session.class);
-if (session.itemExists(targetPath)){
+if (orderBefore) {
+output = reorder(resource, targetPath, session);
+} else if (session.itemExists(targetPath)) {
 if (overwriteTarget && !isDryRun()) {
 
 Review comment:
   i know it's not you, but could you move isDryRun test to after the log is 
emitted?


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:
us...@infra.apache.org


With regards,
Apache Git Services


Re: [VOTE] Release Apache Sling Health Check Core 2.0.0, Sling Health Check Web Console 2.0.0

2019-08-23 Thread Radu Cotescu
+1

> On 22 Aug 2019, at 19:34, Georg Henzler  wrote:
> 
> Please vote to approve this release:
> 
>  [ ] +1 Approve the release
>  [ ]  0 Don't care
>  [ ] -1 Don't release, because ...
> 
> This majority vote is open for at least 72 hours.



[GitHub] [sling-org-apache-sling-pipes] dichaudhary closed pull request #10: SLING-8648

2019-08-23 Thread GitBox
dichaudhary closed pull request #10: SLING-8648
URL: https://github.com/apache/sling-org-apache-sling-pipes/pull/10
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [sling-org-apache-sling-pipes] dichaudhary commented on issue #10: SLING-8648

2019-08-23 Thread GitBox
dichaudhary commented on issue #10: SLING-8648
URL: 
https://github.com/apache/sling-org-apache-sling-pipes/pull/10#issuecomment-524234025
 
 
   closing this in favor of 
https://github.com/apache/sling-org-apache-sling-pipes/pull/11
   
   @npeltier 


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [sling-org-apache-sling-pipes] dichaudhary opened a new pull request #11: SLING-8648 Move pipe to incorporate ordering of the nodes .

2019-08-23 Thread GitBox
dichaudhary opened a new pull request #11: SLING-8648 Move pipe to incorporate 
ordering of the nodes .
URL: https://github.com/apache/sling-org-apache-sling-pipes/pull/11
 
 
   usage ->
   .echo("abc/xyz/def")
   .mv("foo/bar").with("orderBeforeTarget",true)
   
   will move def and its children under foo and place before bar...
   
   @npeltier , please review 


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:
us...@infra.apache.org


With regards,
Apache Git Services