tan-tan-kanarek / github-php-client

MIT License
184 stars 120 forks source link

Should getTheAuthenticatedUser be changed? #76

Closed djplaner closed 8 years ago

djplaner commented 8 years ago

In #75 I included some code that made use of the method getTheAuthenticatedUser and in doing so I identified what I thought was a problem.

As implemented the core of the method is this

return $this->client->request("/user", 'GET', $data, 200, 'GitHubPrivateUser');

The problem is GitHubPrivateUser is only interested/able to use a very narrow subset of the data that GitHub passes back.

GitHubPrivateUser is only interested in

array(
            'total_private_repos' => 'int',
            'owned_private_repos' => 'int',
            'private_gists' => 'int',
            'disk_usage' => 'int',
            'collaborators' => 'int',

Whereas the GitHub API actually passes back a great deal extra information.

When used as I've done with Oauth in #75 it is much more useful to be able to access all of the information passed back. As an interim step I've modified getTheAuthenticatedUser so that it is now doing the following

        return $this->client->request("/user", 'GET', $data, 200, 'GitHubFullUser');

Is this an improvement? Or is it just plain wrong?

tan-tan-kanarek commented 8 years ago

I think it would be fair to reflect Github APIs as they are without applying our own logic in it. I believe that we should return all the information that returned from Github even if we don't think it's needed, mostly because we can't tell what use or abuse other developers would want to get out of it.