reschke commented on code in PR #96:
URL:
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/96#discussion_r1185843290
##########
src/test/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImplTest.java:
##########
@@ -397,8 +397,8 @@ public void mapResourceWithVanityPathsURLTargetNoPath() {
ExpectedMappings.existingResource("/vain-url-nopath")
.singleMapping("/vain-url-nopath")
.singleMappingWithRequest("/app/vain-url-nopath")
- .allMappings("/vain-url-nopath", "")
- .allMappingsWithRequest("/app/vain-url-nopath", "")
Review Comment:
When I added the tests, the goal was to capture the current functionality,
not to reason what the right functionality is. So what we add was adding test
coverage, and then fixing the NPE. That's it.
I understand you believe the behavior is wrong. You are likely right. But I
have zero insight what could break when we change it. IMHO it's safer, not
necessarily "better", to work around this in the calling code.
--
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
