Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change interface of functions to accept steps' parameters #34

Closed
bkielbasa opened this issue Aug 13, 2019 · 1 comment · Fixed by #42
Closed

Change interface of functions to accept steps' parameters #34

bkielbasa opened this issue Aug 13, 2019 · 1 comment · Fixed by #42
Labels
api change it's used when the public API is going to be changed

Comments

@bkielbasa
Copy link
Collaborator

I'm thinking about changing the step's API a bit. Right now it's very simple.

type Func func(ctx context.Context) error

Parameters from steps are fetched using methods like below:

ctx.GetIntParam(0) -> returns the first parameter
ctx.GetIntParam(1) -> returns the second parameter

It works and was super easy to implement but operating on indexes isn't that user-friendly as it should be. I don't want to do much magic but there's an option to extend the interface to be more dynamic.
Using reflection, it's possible to dynamically pass those parameters to the function on the fly so users won't have to play with indexes.

func myCoolStep(ctx Context, int myparam) error {}
// or... 
func myCoolStep(ctx Context, myparam int, anotherparam string) error {}

Thanks to this, the context can be simplified to contain only getters for context-based parameters.

ctx.GetInt(userID{})

On the other hand, it put a bit more responsibility to users while changing/writing scenarios. The debugging can be a bit harder, too. IMO it's acceptable but I want to hear feedback from you.

@bkielbasa bkielbasa added help wanted Extra attention is needed question Further information is requested api change it's used when the public API is going to be changed labels Aug 13, 2019
@bkielbasa
Copy link
Collaborator Author

the decision has been made - we're doing it!

@bkielbasa bkielbasa removed help wanted Extra attention is needed question Further information is requested labels Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api change it's used when the public API is going to be changed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant