paul-bjorkstrand commented on code in PR #41:
URL:
https://github.com/apache/sling-org-apache-sling-engine/pull/41#discussion_r1569266916
##########
src/main/java/org/apache/sling/engine/impl/request/RequestData.java:
##########
@@ -517,6 +518,11 @@ public static void service(SlingHttpServletRequest request,
Servlet servlet = requestData.getContentData().getServlet();
if (servlet == null) {
+ log.warn("Did not find a servlet to handle the request
(path=%s,selectors=%,extension=%s,suffix=%s)",
Review Comment:
A couple things:
1. Shouldn't these just be positional parameters (`{}`) in logs, not
`String.format`-style parameters?
2. Not sure if needed here, but if you use a logger method that ends up
var-args (more than 2 format arguments/exception), it is often good to wrap
with `if (Logger.isXxxEnabled()) { ... }` to avoid the varargs array creation.
For (2) it really depends on how frequently it is expected to occur. This
one might be ok, but I figured better to mention 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
