octokit / go-octokit

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

Potential Missing GitHub API relations #17

Open owenthereal opened 10 years ago

owenthereal commented 10 years ago

This issue is to track missing relations for GitHub API. See this comment for details.

owenthereal commented 10 years ago

@pengwynn I'll keep updating this issue as I go

pengwynn commented 10 years ago

:metal: thanks

technoweenie commented 10 years ago

You can get the pull links from the repository resource. We purposely leave those out (with the exception of user_repositories_url).

owenthereal commented 10 years ago

@technoweenie Yes, I'm aware of getting pull links from repository resource. I was wondering if it makes sense to put it to the root as well. I do have one use case in gh that I don't fetch the repository, get the pull url and create a pull request. I don't need to fetch the repository from GitHub because I'm getting it from git. But I think I could actually use the "pragmatic" layer provided by go-octokit in the future.

technoweenie commented 10 years ago

We've talked about it. Sounds like a pain to maintain at the app level on our end. Now, ideally apps should be caching the hypermedia and all that jazz. But maybe it's worth a little extra effort on our end to make the API clients a little easier to use.

If you want to roll that way, make it obvious which ones you're adding. Maybe add them to the Root object manually.

root := &Root{PullsUrl: Hyperlink("/repos/{owner}/{name}/pulls{/number}"), ...}
// decode the root from the root response to fill in the rest from the API

This way it's obvious where the API client is having to do this extra work. Part of me just wants to make the hypermedia easier to use so that we don't have to maintain duplicate links in a large API site map.

owenthereal commented 10 years ago

@technoweenie It's a great idea to cache the missing hyperlinks on Root. I made a change here.

Now I may have a better idea to implement all these. By default we've a cached root which has all the cached hyperlinks (hard-coded), including those not available from https://api.github.com. People could get a refresh root by making a http call if they want to. And all services are pushed down to the root:

func (c *Client) CachedRoot() *Root {
  return &Root{PullsURL: Hyperlink(xxx), UserURL: Hyperlink(xxx)}
}

func (r *Root) User(m *M) *UserService {
  return &UserService{URL: r.UserURL.Expand(m)}
}

// using cached root
root := client.CachedRoot()
user, result := root.User(octokit.M{"user": "jingweno"}).Get()

// using refreshed root
root := client.Root()
user, result := root.User(octokit.M{"user": "jingweno"}).Get()

This will prob. make it clear what's cached what's not and we only need to cache the root.

What do you think?

technoweenie commented 10 years ago

What if Root() is always cached, and client has a function that can clear the hypermedia cache?

owenthereal commented 10 years ago

@technoweenie I'll create a PR and let's start from there :smile_cat: