-
Notifications
You must be signed in to change notification settings - Fork 194
OwnerType: add OwnerBot #399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
f97a9b2
to
b8b3312
Compare
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/ |
v4 is GraphQL api, which we cannot really support with current design of library. |
This also appears on v3, but unfortunately there's not a single word mentioning it. See |
It does clear some confusion, but makes me wonder whether 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 |
@phadej it's part of github apps (which we still need to PR). But even without other parts of the github apps API, if someone has github app configured, they can use bots to trigger webhooks that will error out in the current github. |
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. |
Looking at this a bit more closely, it does look like the Since it looks like the |
src/GitHub/Data/Definitions.hs
Outdated
@@ -205,7 +205,7 @@ parseOrganization obj = Organization | |||
<*> obj .: "created_at" | |||
|
|||
instance FromJSON User where | |||
parseJSON = mfilter ((== OwnerUser) . userType) . withObject "User" parseUser | |||
parseJSON = withObject "User" parseUser |
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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.
b8b3312
to
0ce3767
Compare
@phadej fixed |
See https://developer.github.com/v4/object/bot/