Contributor
Contributor
I'm very confused. The link is for github api v4 but this library is supporting v3. Is there a shift to support v4? Did I miss the concept of 'bot' defined in the v3 API? I suppose it is just unspecified but should appear somewhere like https://developer.github.com/v3/users/
Contributor
v4 is GraphQL api, which we cannot really support with current design of library.
Contributor Author
Contributor Author
Contributor
It does clear some confusion, but makes me wonder whether v3 stuff works by accident. Could you contact GitHub support and clarify (perfectly resulting into updated v3 documentation).
I said it before, and I repeat, tracking GitHub undocumented features is not worth anything. There should an URL to refer. And this library works with v3 API.
Contributor Author
Contributor Author
Contributor
GitHub Apps are still an API preview. There's been some previous discussion where there's been apprehension about incorporating API previews into this library in #351 and #367.
I started work on a separate library for API previews but haven't had any time to work on it recently. You're welcome to contribute or use that code for inspiration. I might try to clean up some of it this weekend. You'll probably also find #365 and #370 relevant for authenticating as a GitHub App.
Contributor
Looking at this a bit more closely, it does look like the Bot user type has made its way into a few different parts of the v3 API. Here's another example where .merged_by.type is Bot. It looks like we're getting lucky on this occasion because SimpleUser doesn't have a type field otherwise this library would fail to decode some pull requests.
Since it looks like the Bot type is here to stay in the v3 API, it seems like a good idea to make this change otherwise we'll eventually end up with users raising issues because they're getting Unknown OwnerType errors.
|
|
||
| instance FromJSON User where | ||
| parseJSON = mfilter ((== OwnerUser) . userType) . withObject "User" parseUser | ||
| parseJSON = withObject "User" parseUser |
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd still check it's not an OwnerOrganisation.
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I think this change is unavoidable. I'll improve the haddocks fo OwnerType after this is merged, yet there's one small issue in the code.
Contributor Author
