Proposal: Add GetByID methods to UsersService, RepositoriesService, with caveat.

[dmitshur]

I'd like to hear your thoughts on the following proposal.

I know this project is Go client library for accessing the GitHub API. It tries to stay very current with GitHub API changes. It implements support for new APIs early, even when they're still in preview period and may result in changes before preview period ends.

As a result, this Go package tries to maintain a stable API, but when breaking GitHub API changes happen that necessitate otherwise, it follows suit.

I propose considering adding two methods, but read below for details:

// GetByID fetches a user.
func (s *UsersService) GetByID(id int) (*User, *Response, error) { ... }

// GetByID fetches a repository.
func (s *RepositoriesService) GetByID(id int) (*Repository, *Response, error) { ... }

Motivation

Right now, UsersService exposes two methods for getting users, one is by login, and the other gets the authenticated user.

It's possible to change usernames on github.com, so login is not necessary a very permanent way to look up the same user. As far as I know, IDs can not ever be changed.

Caveat

Both of these methods are not definitely described as part of the GitHub API on https://developer.github.com/v3/ from what I can tell.

Because of that, simply providing these methods would be completely unacceptable, because there is a higher than usual chance they may change or go away in the future.

However, I consider them to be quite close in nature to the new API endpoints that are a part of a preview period (that require setting specific media type Accept header values to opt in). So, if these methods are provided with a comment saying:

// Note: GetByID uses the undocumented GitHub API endpoint /user/:id.

Then it might be acceptable, since users of the API can see if the benefits of using this method (for their needs) is worth accepting a risk of potentially needing to update code if there are changes in the future.

However, both API endpoints do work, and have worked for at least a year without any changes, from what I know.

E.g.:

https://api.github.com/users/octocat - get user with username "octocat", it says user ID is 583231
https://api.github.com/user/583231 - gets the same user by ID

Also note, it's possible to get a user by ID in a following way (this uses fully documented APIs only, however it returns a smaller set of user details, but a second API call to get user by login can be made if neccessary):

// gitHubUsersGetByID fetches a GitHub user based on their userID.
func gitHubUsersGetByID(gh *github.Client, userID int) (*github.User, *github.Response, error) {
    req, err := gh.NewRequest("GET", fmt.Sprintf("/users?since=%v&per_page=1", userID-1), nil)
    if err != nil {
        return nil, nil, err
    }
    var users []github.User
    resp, err := gh.Do(req, &users)
    if err != nil {
        return nil, resp, err
    }
    if len(users) != 1 {
        return nil, resp, fmt.Errorf("expected 1 user, got %v users", len(users))
    }
    if users[0].ID == nil {
        return nil, resp, fmt.Errorf("got user with nil user ID: %#v", users[0])
    }
    if *users[0].ID != userID {
        return nil, resp, fmt.Errorf("expected user ID %v, got user ID %v", userID, *users[0].ID)
    }
    return &users[0], resp, nil
}

Implementation details

See the following commits for implementation details.

UsersService.GetByID - https://github.com/sourcegraph/go-github/commit/e4f201ff9b2966d65e34b342d19a386d5c35624f

RepositoriesService.GetByID - https://github.com/sourcegraph/go-github/commit/da394ef1f53de34e9c3e5782ca27589df32b426c

If this proposal is accepted, I can clean them up and make PRs.

What do you think?

[willnorris]

Yeah, this sounds good to me.

Quinn had started something similar to this in #68, though I think we're way past the point of being able to make the kind of breaking changes that proposes, so I think what you have here is a good compromise. I can't find it in that issue, but I seem to recall somewhere hearing that internally GitHub actually maps the name based endpoints to the ID based endpoints, so these actually should be pretty stable. That said, since they're not publicly documented, having some cautionary text in the method doc seems prudent.

[dmitshur]

Sounds great, I will make 2 PRs.

[dmitshur]

I can't find it in that issue, but I seem to recall somewhere hearing that internally GitHub actually maps the name based endpoints to the ID based endpoints, so these actually should be pretty stable.

I've recently seen some internal API URLs in responses from GitHub Notifications API point to these endpoints.

Edit: I've also since discovered #60 (comment) which confirms these endpoints can be relied on.