rombert commented on code in PR #165:
URL: 
https://github.com/apache/sling-org-apache-sling-feature-cpconverter/pull/165#discussion_r1173510075


##########
src/main/java/org/apache/sling/feature/cpconverter/cli/ContentPackage2FeatureModelConverterLauncher.java:
##########
@@ -145,6 +146,9 @@ public final class 
ContentPackage2FeatureModelConverterLauncher implements Runna
 
     @Option(names = { "--sling-initial-content-policy" }, description = 
"Determines what to do with Sling-Initial-Content found in embedded bundles. 
Valid values: ${COMPLETION-CANDIDATES}.", required = false, showDefaultValue = 
Visibility.ALWAYS)
     private SlingInitialContentPolicy slingInitialContentPolicy = 
SlingInitialContentPolicy.KEEP;
+    
+    @Option(names = { "--runmode-policy" }, description = "Determines how to 
determine the final runmode of an artifac. Valid values: 
${COMPLETION-CANDIDATES}.", required = false, showDefaultValue = 
Visibility.ALWAYS)

Review Comment:
   Typo:
   
   ```suggestion
       @Option(names = { "--runmode-policy" }, description = "Determines how to 
determine the final runmode of an artifact. Valid values: 
${COMPLETION-CANDIDATES}.", required = false, showDefaultValue = 
Visibility.ALWAYS)
   ```



##########
src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractRegexEntryHandler.java:
##########
@@ -45,4 +54,39 @@ public final boolean matches(@NotNull String path) {
         return pattern;
     }
 
+    protected String extractTargetRunmode(String path, 
ContentPackage2FeatureModelConverter converter,
+            String runMode, String runModeMatch) {
+                String targetRunmode;
+                if  
(RunmodePolicy.PREPEND_INHERITED.equals(converter.getRunmodePolicy())) {
+                    final List<String> runModes = new ArrayList<>();
+                    final List<String> inheritedRunModes = runMode == null ? 
Collections.emptyList() : Arrays.asList(StringUtils.split(runMode, '.'));
+            
+                    runModes.addAll(inheritedRunModes);
+                    // append found runmodes without duplicates (legacy 
behavior direct_only established by appending to empty List)
+                    if (StringUtils.isNotEmpty(runModeMatch)) {
+                        // there is a specified RunMode
+                        logger.debug("Runmode {} was extracted from path {}", 
runModeMatch, path);
+                        List<String> newRunModes = 
Arrays.asList(StringUtils.split(runModeMatch, '.'));
+            
+                        // add only new RunModes that are not already present
+                        List<String> newRunModesList = newRunModes.stream()
+                                                                  .filter(mode 
-> !runModes.contains(mode))
+                                                                  
.collect(Collectors.toList());
+            
+                        // identify diverging list of runmodes between parent 
& direct definition as diverging criteria between runmode policies
+                        if(!runModes.isEmpty() && 
!CollectionUtils.isEqualCollection(newRunModes, inheritedRunModes)) {
+                            logger.info("Found diverging runmodes list {} 
diverging from defined runmodes on the parent {}", newRunModes.toString(), 
inheritedRunModes.toString());

Review Comment:
   1. I don't think you need `toString()` here.
   2. Can we clarify which runModes are going to be used?
   3. Should we add a callout to set the runmodePolicy explicitly if the 
results are not as expected?



##########
src/main/java/org/apache/sling/feature/cpconverter/handlers/AbstractConfigurationEntryHandler.java:
##########
@@ -45,11 +44,13 @@ void setEnforceConfigurationBelowConfigFolder(boolean 
enforceConfigurationBelowC
     }
 
     @Override
-    public final void handle(@NotNull String path, @NotNull Archive archive, 
@NotNull Entry entry, @NotNull ContentPackage2FeatureModelConverter converter) 
throws IOException, ConverterException {
+    public final void handle(@NotNull String path, @NotNull Archive archive, 
@NotNull Entry entry, @NotNull ContentPackage2FeatureModelConverter converter, 
String runMode) throws IOException, ConverterException {
 
         Matcher matcher = getPattern().matcher(path);
         
-        String runMode;
+        String targetRunmode;
+        

Review Comment:
   Nit: too many empty lines.



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

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to