rombert commented on a change in pull request #53:
URL: 
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/53#discussion_r761946932



##########
File path: pom.xml
##########
@@ -149,6 +149,11 @@
             <artifactId>annotations</artifactId>
             <scope>provided</scope>
         </dependency>
+        <dependency>

Review comment:
       Can we make this dependency optional? I am worried that this will lead 
in a wave of dependency breakages in various consumer projects.

##########
File path: pom.xml
##########
@@ -149,6 +149,11 @@
             <artifactId>annotations</artifactId>
             <scope>provided</scope>
         </dependency>
+        <dependency>
+            <groupId>org.apache.sling</groupId>
+            <artifactId>org.apache.sling.commons.metrics</artifactId>
+            <version>1.2.8</version>

Review comment:
       The scope should be provided.

##########
File path: 
src/test/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImplTest.java
##########
@@ -94,6 +95,7 @@ public ResourceMapperImplTest(boolean 
optimiseAliasResolution) {
     @Before
     public void prepare() throws LoginException {
 
+        ctx.registerService(ResourceResolverMetrics.class, 
Mockito.mock(ResourceResolverMetrics.class));

Review comment:
       I think we shouldn't mix Sling mocks and mockito here. Can you just pass 
the plain object?




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to