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

Add kwargs to public methods #57

Merged
merged 1 commit into from
Mar 14, 2022

Conversation

nurikk
Copy link
Contributor

@nurikk nurikk commented Mar 14, 2022

add kwargs to public methods. so we wouldn't get errors when using object spreading
ex: TypeError: matrix() got an unexpected keyword argument 'extra_param1'

#55
#49

Copy link
Contributor

@TimMcCauley TimMcCauley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, many thanks!

@TimMcCauley TimMcCauley merged commit 5e00333 into gis-ops:master Mar 14, 2022
@nilsnolde
Copy link
Member

@nurikk actually this doesn't do anything as far as I can see.. you don't use the kwargs in the methods so they're accepted, yes, but the request won't use them right?

that's not really consistent with how the library works for other providers (ie the PRs you linked), so I'd ask to please implement using the kwargs as well. could you also copy/paste some test lines from those PRs?

@TimMcCauley
Copy link
Contributor

True, they are not being used in the request itself. @nurikk I'm assuming these are some custom set parameters that are being used to keep a mapping of the request and response itself? Or what was your intention here?

@TimMcCauley
Copy link
Contributor

Reverted the changes for now.

@nurikk
Copy link
Contributor Author

nurikk commented Mar 15, 2022

Created a new PR #59

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants