Skip to content

Conversation

adamziel
Copy link
Collaborator

@adamziel adamziel commented Apr 24, 2023

Description

This Pull Request proposes a change in the structure of Blueprint JSON definitions and step handlers to make them consistent with each other.

In the current implementation, there is a discrepancy between how steps are defined in the Blueprint JSON and how their corresponding handlers are defined. While the Blueprint step definition includes the step name and its associated parameters, the handler function directly accepts the parameters without any reference to the step name. This lack of consistency can lead to confusion and errors while developing and maintaining the Blueprint.

This PR refactors step handler functions to accept parameters as an object that maps one to one with the related JSON structure. It also reuses the same TypeScript types. This makes the code more intuitive and less error-prone by providing a clear mapping between the step definitions and their corresponding handlers.

Before

// login Blueprint step: { "step": "login", "username": "admin", "password": "123" } // login handler progressTracker.setCaption( "Logging in" ) login( playground, "admin", "password" );

After

// login Blueprint step: { "step": "login", "username": "admin", "password": "123" } // login handler login( playground, { "username": "admin", "password": "123" }, { tracker: progressTracker } );
@dmsnell
Copy link
Member

dmsnell commented Apr 24, 2023

Really appreciate the cross-platform harmonization of calling convention here. That can go pretty far to bridging any gaps between the different environments.

@adamziel
Copy link
Collaborator Author

I'm glad you like it @dmsnell! I just pushed an update that refactors all the steps, I wonder if you can see any possible improvements there

@adamziel adamziel changed the title Explore: Align step handlers with JSON definitions Use the same structure for Blueprint JSON definitions and step handlers Apr 25, 2023
@adamziel adamziel self-assigned this Apr 25, 2023
@adamziel adamziel marked this pull request as ready for review April 25, 2023 17:27
@adamziel
Copy link
Collaborator Author

Let's merge this one. I'm happy to apply any feedback as a follow-up PR.

@adamziel adamziel merged commit 3269e9e into trunk Apr 25, 2023
@adamziel adamziel deleted the align-step-handlers-with-json-declarations branch April 25, 2023 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

2 participants