[GitHub] [sling-org-apache-sling-resourceresolver] anchela commented on a change in pull request #20: SLING-9676 - Improve Javadoc for MapEntriesHandler

[GitBox <gi...@apache.org>]

anchela commented on a change in pull request #20:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/20#discussion_r474431964



##########
File path: src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntry.java
##########
@@ -268,7 +267,12 @@ public MapEntry(String url, final int status, final boolean trailingSlash,
         this.order = order;
     }
 
-    // Returns the replacement or null if the value does not match
+    /**
+     * Replaces the specified value according to the rules of this entry
+     * 
+     * @param value the value to replace
+     * @return a replaced value of <code>null</code> if the value does not match
+     */
     public String[] replace(final String value) {

Review comment:
       while adding nullability annotations is not in the scope of the original jira ticket, i am very glad to see you added them for the MapEntriesHandler. imo they also help improve readability of the code (apart from automatic nullability violations being spotted and helping to prevent errors)....
   
   so, maybe adding '@Nullable' for the return value and '@NotNull' with the value param would be a good thing.... but as i said: i was mainly asking for javadoc and this would to just an added benefit. but if it leaves the MapEntry class in a odd state because the annotations are only added in some places, feel free to just ignore this comment.




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