juanpablo-santos commented on a change in pull request #141:
URL: https://github.com/apache/jspwiki/pull/141#discussion_r732747456



##########
File path: 
jspwiki-event/src/main/java/org/apache/wiki/event/WikiSecurityEvent.java
##########
@@ -112,7 +112,7 @@ Licensed to the Apache Software Foundation (ASF) under one
     public static final int   PROFILE_NAME_CHANGED     = 54;
     
     /** The security logging service. */
-    protected static final Logger log = LogManager.getLogger( "SecurityLog" );
+    static final Logger log = LogManager.getLogger( "SecurityLog" );

Review comment:
       may be making it `private static final` here? loggers are usually 
defined that way

##########
File path: jspwiki-main/src/main/java/org/apache/wiki/WikiSession.java
##########
@@ -88,7 +88,7 @@ Licensed to the Apache Software Foundation (ASF) under one
      * @param group the group to test
      * @return the result
      */
-    protected boolean isInGroup( final Group group ) {
+    boolean isInGroup( final Group group ) {

Review comment:
       JSPWiki public API [allows to inject a custom `Session` 
implementation](https://jspwiki-wiki.apache.org/Wiki.jsp?page=JSPWikiPublicAPI#section-JSPWikiPublicAPI-ProvidingCustomCoreAPIImplementations),
 so in this case I think it makes more sense to remove the `final` attribute 
from the class, rather than removing the `protected` modifier

##########
File path: 
jspwiki-main/src/main/java/org/apache/wiki/ui/DefaultCommandResolver.java
##########
@@ -231,7 +231,7 @@ public String getSpecialPageReference( final String page ) {
      * @param request the HTTP request
      * @return the resolved Command, or <code>null</code> if not found
      */
-    protected Command extractCommandFromPath( final HttpServletRequest request 
) {
+    Command extractCommandFromPath( final HttpServletRequest request ) {

Review comment:
       A custom `CommandResolver` can be used, so in this case I think it makes 
more sense to remove the `final` modifier on the class and keep the `protected` 
modifier elsewhere.




-- 
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...@jspwiki.apache.org

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


Reply via email to