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

Optional PR: Move optional params to object (WIP) #22

Closed
wants to merge 3 commits into from

Conversation

tony
Copy link
Contributor

@tony tony commented Dec 3, 2021

This avoids the signature becoming unwieldy in the event future params are introduced. Improvement on top of #15

allowIssueWriting and artifactName are subject to this

When sending object literals we can know the param being passed by name, and also use them with minimal bloat thanks to destructuring.

Includes #21

@tony tony force-pushed the artifactName-to-options-object branch 4 times, most recently from 78d624c to 08932d3 Compare December 3, 2021 22:28
@tony tony changed the title Optional PR: Move artifactName to object Optional PR: Move optional params to object Dec 3, 2021
This fixes multiple issues posting during subsequent run(s)
when artifactName was specified.

Signed-off-by: Tony Narlock <[email protected]>
By changing artifactName to be in an object before before releasing
we avoid positional, unnamed arguments that can cause confusion
when the signature is extended.

The curly brackets are destructuring assignment.

Signed-off-by: Tony Narlock <[email protected]>
Fixes the same issue of avoiding confusion with parameters are
added to the signature.

Signed-off-by: Tony Narlock <[email protected]>
@tony tony force-pushed the artifactName-to-options-object branch from 08932d3 to 904a39f Compare December 4, 2021 00:12
@tony tony changed the title Optional PR: Move optional params to object Optional PR: Move optional params to object (WIP) Dec 4, 2021
@tony tony force-pushed the artifactName-to-options-object branch from f876682 to 904a39f Compare December 6, 2021 17:04
@tony
Copy link
Contributor Author

tony commented Feb 22, 2022

Closing this for now as time has passed. I like this change and heavily prefer objects over positional arguments for clarity avoiding refactors when options are added/deprecated

@tony tony closed this Feb 22, 2022
@tony tony deleted the artifactName-to-options-object branch February 22, 2022 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant