rmcdouga commented on code in PR #3:
URL: 
https://github.com/apache/sling-org-apache-sling-testing-hamcrest/pull/3#discussion_r1391551624


##########
pom.xml:
##########
@@ -55,7 +55,7 @@
         <dependency>
             <groupId>org.apache.sling</groupId>
             <artifactId>org.apache.sling.api</artifactId>
-            <version>2.4.0</version>
+            <version>2.22.0</version>

Review Comment:
   I believe I had to update this because of the upgrade to sling-mock 3.4.14.  
That version of the project depends on sling.api 2.22.0
   
   Without upgrading the sling.api 2.22.0, the project will not compile because 
of a java.lang.NoClassDefFoundError: 
org/apache/sling/api/wrappers/DeepReadModifiableValueMapDecorator error.
   
   While I understand the sentiment behind using the lowest compatible version, 
I'm not sure I agree with it.  In practical terms, this doesn't really affect 
existing users.  If someone can't or doesn't want to upgrade their sling.api 
dependency, then they can always remain on the older version of this library.  
If they want or need to use the latest version, then the price of admission is 
updating their dependencies.  No-one is forcing this on anyone.
   
   Not upgrading though (for whatever reason) should not be a best practice (at 
least IMHO, which I admit lacks experience in the sling project).  Everyone 
should be upgrading their dependencies regularly.  If upgrading is a problem, 
then delay the upgrade until you can figure out what else needs to be changed, 
but I see upgrading eventually is inevitable so there's no point is putting it 
off too long.
   
   My preference would be to not enable the bad practice of having outdated 
dependencies.
   
   Having said that, if you feel strongly about this, it will take some work 
but I can keep rolling things back until I have a version that works, but I 
won't feel good about it. 😄 



##########
pom.xml:
##########
@@ -55,7 +55,7 @@
         <dependency>
             <groupId>org.apache.sling</groupId>
             <artifactId>org.apache.sling.api</artifactId>
-            <version>2.4.0</version>
+            <version>2.22.0</version>

Review Comment:
   I believe I had to update this because of the upgrade to sling-mock 3.4.14.  
That version of the project depends on sling.api 2.22.0
   
   Without upgrading the sling.api 2.22.0, the project will not compile because 
of a java.lang.NoClassDefFoundError: 
org/apache/sling/api/wrappers/DeepReadModifiableValueMapDecorator error.
   
   While I understand the sentiment behind using the lowest compatible version, 
I'm not sure I agree with it.  In practical terms, this doesn't really affect 
existing users.  If someone can't or doesn't want to upgrade their sling.api 
dependency, then they can always remain on the older version of this library.  
If they want or need to use the latest version, then the price of admission is 
updating their dependencies.  No-one is forcing this on anyone.
   
   Not upgrading though (for whatever reason) should not be a best practice (at 
least IMHO, which I admit lacks experience in the sling project).  Everyone 
should be upgrading their dependencies regularly.  If upgrading is a problem, 
then delay the upgrade until you can figure out what else needs to be changed, 
but I see upgrading eventually is inevitable so there's no point is putting it 
off too long.
   
   My preference would be to not enable the bad practice of having outdated 
dependencies.
   
   Having said that, if you feel strongly about this, it will take some work 
but I can keep rolling things back until I have a version that works, but I 
won't feel good about it. 😄 



-- 
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: dev-unsubscr...@sling.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to