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

Use args ...Arguments instead of args Arguments? #74

Closed
TJM opened this issue Nov 1, 2020 · 1 comment
Closed

Use args ...Arguments instead of args Arguments? #74

TJM opened this issue Nov 1, 2020 · 1 comment

Comments

@TJM
Copy link
Contributor

TJM commented Nov 1, 2020

Hi, I found your module when trying to "publish" TJM/go-trello (fork from VojtechVitek/go-trello ) that I hacked on for several days. I like a lot of the things you have done here, and I am ready to switch over. I was actually working on switching my (real) project over to use your module instead and it occurred to me...

Couldn't you swap all the args Arguments for args ...Arguments, thus making the "trello.Defaults()" on every call unnecessary (optional)? It may not be exactly that, but something like that? The go-trello code that I "forked" had Argument as a struct with Name and Value, then []Arguments or ...Argument :-/ ... just checking as that feels a bit strange to generate a new/empty map for every call. At the very least, I am thinking create it once at the top and use a variable for each one? 🤷‍♂️

I am fairly new to golang, so I am not sure what the ramifications of that are?

Also, Based on my findings trying to hack go-trello into submission, I think I can offer several PRs to add capability/functionality if you are interested.

One of the biggest epiphanies I had was the "AddMemberResponse" is actually a (partial) "Board" response... of course you have to have []*Memberships and []*Members in your Board type. Anyhow, lets focus on the "optionalization" (new word?) of the args for this issue. :)

~tommy

EDIT: I finally found the right word "variadic" :)

https://www.geeksforgeeks.org/golang-program-that-uses-func-with-variable-argument-list/

At the most basic level, you could just use args ...Arguments and then add a [0] to the places where you access args now, but of course the ... might encourage people to pass more than one list of arguments, so you should maybe come up with some way to "flatten" them? It seems like you already had some "manual" flattening going on (like "if pos, then set pos")... so it would already be useful IMO.

@TJM
Copy link
Contributor Author

TJM commented Nov 5, 2020

As noted, check my PR #78 for the "fix" for this.

@adlio adlio closed this as completed in cc656c6 Nov 16, 2020
adlio added a commit that referenced this issue Nov 16, 2020
Fix #74 - make extra arguments optional
adlio added a commit that referenced this issue Mar 28, 2021
* 'master' of github.com:adlio/trello:
  Fix #74 - make extra arguments optional
  feat: Add IDOrganizations and IDBoards to Member
  Use rate.Limiter for throttling
  Add GetMyMember function
  Add GetChecklist() method
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

No branches or pull requests

1 participant