octokit / go-octokit

Simple Go wrapper for the GitHub API
https://github.com/octokit/go-octokit
MIT License
258 stars 79 forks source link

Stop exposing `All` and `One` methods #119

Open calavera opened 8 years ago

calavera commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

:+1: 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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 3 years ago

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