You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "jhorvath (via GitHub)" <gi...@apache.org> on 2024/06/10 06:47:42 UTC

[PR] Update dependencies for OCI Vault integration and clean up multistep dialogs [netbeans]

jhorvath opened a new pull request, #7453:
URL: https://github.com/apache/netbeans/pull/7453

   - The command for storing DB connection detail to the OCI Vault now also updates project dependencies. 
   - Remove duplicates in multistep dialogs implementation.


-- 
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: notifications-unsubscribe@netbeans.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Update dependencies for OCI Vault integration and clean up multistep dialogs [netbeans]

Posted by "sdedic (via GitHub)" <gi...@apache.org>.
sdedic commented on code in PR #7453:
URL: https://github.com/apache/netbeans/pull/7453#discussion_r1632707912


##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/assets/Steps.java:
##########
@@ -98,11 +100,82 @@ public CompletableFuture<Object> executeMultistep(Step firstStep, Lookup lookup)
             if (DialogDescriptor.OK_OPTION == dd.notify(ci)) {
                 future.complete(multistep.getResult());
             } else {
-                future.complete(null);
+                future.cancel(true);
             }
         });
         return future;
     }
+    
+    /**
+     * Provider class that supplies the next {@link Step} for navigation from the current {@link Step}.
+     * 
+     */
+    public static class NextStepProvider {

Review Comment:
   nitpick: final
   
   Other thoughts -- I don't know where this Provider is leading to. The "ultimate" goal may be just an utility usabe here, or something eventually reusable.
   
   In the code, it is used to specify a (complete) sequence of panels; the panel instances are created at one place, in advance ad fed to the Builder. That's OK, if no variability is ever expected (everything created upfront), but in SuggestedItemStep, the current step's getValue() is used.
   
   I would suggest to *defer* the instance creation to the time the user is actually advancing to the next step: provide a Factory for the step instance, rather than step instance itself.  I'd suggest to reshape the Provider to have a `Function<Step, Step>` instead of pre-created Step instance: this way the logic can be moved to `AddNewAsset` that has the overall control over the multi-step input. It can provide
   ```
   builder.stepForClass(CompartmentStep.class, (s) -> new SuggestedStep(s.getValue()))
   ```
   It's also a question if the Provider should work based on currenct step's Class, or just based on step's position - currently no alternate path selection is used.
   



##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/assets/Steps.java:
##########
@@ -358,11 +427,14 @@ public NotifyDescriptor createInput() {
 
         @Override
         public Step getNext() {
-            Step next = context.getNextStep();
-            if (next != null) {
-                next.prepare(getValue(), lookup);
-            }
-            return next;
+            NextStepProvider nsProvider = lookup.lookup(NextStepProvider.class);

Review Comment:
   Minor: Step could be an `abstract` class with this as the default impl of `getNext()`. Or `Step` interace can implement this in its default method



-- 
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: notifications-unsubscribe@netbeans.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Update dependencies for OCI Vault integration and clean up multistep dialogs [netbeans]

Posted by "jhorvath (via GitHub)" <gi...@apache.org>.
jhorvath commented on code in PR #7453:
URL: https://github.com/apache/netbeans/pull/7453#discussion_r1633055575


##########
enterprise/cloud.oracle/src/org/netbeans/modules/cloud/oracle/assets/Steps.java:
##########
@@ -358,11 +427,14 @@ public NotifyDescriptor createInput() {
 
         @Override
         public Step getNext() {
-            Step next = context.getNextStep();
-            if (next != null) {
-                next.prepare(getValue(), lookup);
-            }
-            return next;
+            NextStepProvider nsProvider = lookup.lookup(NextStepProvider.class);

Review Comment:
   This is good idea, but we don't have a way how to supply lookup to the default implementation for now.



-- 
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: notifications-unsubscribe@netbeans.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Update dependencies for OCI Vault integration and clean up multistep dialogs [netbeans]

Posted by "jhorvath (via GitHub)" <gi...@apache.org>.
jhorvath merged PR #7453:
URL: https://github.com/apache/netbeans/pull/7453


-- 
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: notifications-unsubscribe@netbeans.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists