kwin commented on a change in pull request #80:
URL: 
https://github.com/apache/jackrabbit-filevault/pull/80#discussion_r430168193



##########
File path: 
vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/registry/impl/AbstractPackageRegistry.java
##########
@@ -59,6 +65,17 @@
      */
     public static final String DEFAULT_PACKAGE_ROOT_PATH_PREFIX = 
DEFAULT_PACKAGE_ROOT_PATH + "/";
 
+    protected final String[] additionalAuthorizableIdsAllowedToExecuteHooks;
+
+    protected final String[] 
additionalAuthorizableIdsAllowedToInstallPackagesRequiringRoot;

Review comment:
       Indeed, but I like verbosity over short names. Do you have any other 
proposal?

##########
File path: 
vault-core/src/main/java/org/apache/jackrabbit/vault/packaging/impl/JcrPackageManagerImpl.java
##########
@@ -88,8 +88,8 @@
      * @param session repository session
      * @param roots the root paths to store the packages.
      */
-    public JcrPackageManagerImpl(Session session, String[] roots) {
-        this(new JcrPackageRegistry(session, roots));
+    public JcrPackageManagerImpl(@NotNull Session session, @Nullable String[] 
roots, @Nullable String[] additionalAuthorizableIdsAllowedToExecuteHooks, 
@Nullable String[] 
additionalAuthorizableIdsAllowedToInstallPackagesRequiringRoot) {
+        this(new JcrPackageRegistry(session, 
additionalAuthorizableIdsAllowedToExecuteHooks, 
additionalAuthorizableIdsAllowedToInstallPackagesRequiringRoot, roots));

Review comment:
       For me this is not API, as this is in a not exported impl package, so 
keeping backwards-compatibility is not necessary as this can only be used by 
the bundle itself. But wrapping the config makes sense.




----------------------------------------------------------------
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:
[email protected]


Reply via email to