[GitHub] [jackrabbit-oak] anchela commented on a diff in pull request #537: allow mapping of complex user properties that implement the Supplier interface

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

anchela commented on code in PR #537:
URL: https://github.com/apache/jackrabbit-oak/pull/537#discussion_r844762388


##########
oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/basic/DefaultSyncContextTest.java:
##########
@@ -1201,6 +1201,36 @@ public void testSyncPropertiesRemapped() throws Exception {
         }
     }
 
+    @Test

Review Comment:
   minor wish: i don't do super consistently either but having a javadoc above the test inciting for which JIRA ticket this test was added is usually helpful if one needs to investigate later on.
   since a dedicated jira ticket is anyway needed for the commit message you may want to add it as a comment here at the same time.



##########
oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/TestIdentityProvider.java:
##########
@@ -283,4 +285,20 @@ public ForeignExternalGroup() {
             return ImmutableList.of();
         }
     }
+    public static class UserWithSupplierProperties extends TestUser {

Review Comment:
   trivial nitpicking: there should be an extra line separating the UserWithSupplierProperties from the previous class.



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

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