Skip to content
This repository has been archived by the owner on Jun 20, 2022. It is now read-only.

Stop exposing All and One methods #119

Open
1 task
calavera opened this issue Aug 7, 2015 · 7 comments
Open
1 task

Stop exposing All and One methods #119

calavera opened this issue Aug 7, 2015 · 7 comments

Comments

@calavera
Copy link
Contributor

calavera commented Aug 7, 2015

Users should not need to know how go-octokit generates hypermedia urls internally. The fact that people must provide maps with aleatory keys to those methods to call the api is a bad smell. Octokit should provide high level functions to perform every operation. Something like:

client.Repositories().Get("octokit", "go-octokit")
client.Repositories().Fork("octokit", "go-octokit")

And so on.

I'll open a PR soon with a few of those methods, but providing all of them can be a community effort.
Feel free to claim resources that you want to modify yourself commenting in this issue.

TODO:

@pengwynn
Copy link
Collaborator

I dig this approach, @calavera.

Stop exposing All and One methods

Maybe

Stop exposing All and One methods exclusively?

I think there's some value in keeping these public for extensibility.

I'll open a PR soon with a few of those methods, but providing all of them can be a community effort.

Looking forward to seeing your approach.

@half-ogre
Copy link
Contributor

👍 To @calavera's proposal of adding "friendly" functions that don't need the map, wether or not the current map-needy functions remain.

@half-ogre
Copy link
Contributor

@pengwynn, @calavera: Any interest in me taking one of the API endpoint groups and sending a PR with what this might look like, so you can make a call on whether to go down this path? I'm happy to help with the work, but don't want to do too much of it before a decision is made.

If so, what part of the API would you like me to prototype this with?

@pengwynn
Copy link
Collaborator

pengwynn commented Dec 5, 2015

Any interest in me taking one of the API endpoint groups and sending a PR with what this might look like, so you can make a call on whether to go down this path? I

@half-ogre Absolutely!

If so, what part of the API would you like me to prototype this with?

I don't think it matters much, but Issues might be a good place to start.

@calavera
Copy link
Contributor Author

calavera commented Dec 7, 2015

Please, feel free to carry this code in any way. Unfortunately, I'm very busy with other "small" open source project to polish this how I'd like it.

@JaredReisinger
Copy link
Contributor

So, coincidentally, I'm using go-octokit for a project I'm working on and just happen to need the "team" APIs... so I thought I'd contribute. Once of the things I noticed as I was working on a non-One/All implementation was that I was re-writing a lot of boilerplate code (basically, the stuff that's already in One() and All()). Rather than doing this, would it make the most sense to keep the One() and All() methods, but just add the "nice" named wrappers around them? For example, octokit.Teams().GetMembers(teamId) would just call octokit.Users().All(Hyperlink("/teams/{id}/members"), M{"id": teamId}) under the covers. Is that a reasonable approach?

@ghost
Copy link

ghost commented Jan 24, 2021

Yes I think this should be put into the API. Pin the issue?

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

No branches or pull requests

4 participants