Support PubSubHubbub API for creating hooks

[tomasaschan]

Are there any plans to support the PubSubHubbub API for creating webhooks?

I could try to create a PR for implementing this (I'm hoping to use it for implementing https://github.com/terraform-providers/terraform-provider-github/issues/275) if there is any chance of it being accepted.

[gmlewis]

Since it is a part of the GitHub Developer v3 API, it makes sense to me to support it in this repo.

If there are no objections from @willnorris or @gauntface then I say we move forward with a design and PR.

[tomasaschan]

@gmlewis Great! Is there any design documents or similar that I should submit for review before starting to work on the actual code? (Also, are there any specific build instructions for this repo I should know about?)

[gmlewis]

We have no formal design docs but you should definitely read through and be familiar with CONTRIBUTING.md.

It would be great if you could informally describe your design here in this issue including generally how you plan to implement the new feature and give other contributors and users an opportunity to comment before getting too deep into the PR.

[tomasaschan]

All the infrastructure for handling authentication etc seems to already be handled, so I figure I can just look at how other API calls are handled, and mimic that. For example, the create webhook code seems simple enough to imitate.

To avoid cluttering up existing code, I figure I'll place it in a new file github/repos_pubsubhubbub.go (with unit tests next to it), and add the functions to the RepositoryService type.

Does that sound OK?

[gmlewis]

That sounds like a great plan to me. Hopefully if others have any feedback they will respond to this thread earlier rather than later. 😂

[wesleimp]

@gmlewis this issue can be closed

[gmlewis]

It seems to me like this issue is still open, @wesleimp ... why do you say this one can be closed?

[wesleimp]

@gmlewis you are correct, sorry my bad.

[gmlewis]

No problem, @wesleimp ! Thank you for all your help! It is greatly appreciated!

[joshuabezaleel]

Hi @gmlewis , I stumbled upon this issue while browsing the repo and I am interested if somehow this issue is still available 🙂

[gmlewis]

Yes, since we have not heard from @tomasaschan since last August, I think we can safely assume that this issue is available.

Thank you, @joshuabezaleel ! I'll assign it to you.

[joshuabezaleel]

Hi @gmlewis , is it okay if I ask a particular technical question here?

For DeleteHook testing, does it mean that we don't have to check whether the statusCode of the response is 204 or not?
I tried to issue the PubSubHubBub request using curl and it returns 204 with proper parameters and 422 with invalid parameters but I kept getting 200 while firing it in the testing part of the code.

Thank you very much in advance! 🙂

[gmlewis]

If I understand correctly, you are asking about the unit test... Correct, you don't need to fake (or test) the 204 explicitly... it is sufficient to just fake/test the 200. You can modify the test if you wish, but the unit tests are mainly concerned with getting the full round-trip exercised, and the 200 is sufficient.

Thank you, @joshuabezaleel !

[joshuabezaleel]

Hi @gmlewis , I was actually thinking of this since the resp.statusCode would be different if I leave some required parameter empty, that is hub.mode and hub.topic, or not so I don't know whether I should cover this for the test.

I've put up a PR and I think the code is still not good enough and really subject to change. Sorry for the troublesome in advance. Looking forward to the review! 🙂

[gmlewis]

This would be a great PR for any new contributor to this repo or a new Go developer.
All contributions are greatly appreciated!

Feel free to volunteer for any issue and the issue can be assigned to you so that others don't attempt to duplicate the work.

Please check out our CONTRIBUTING.md guide to get started. (In particular, please remember to go generate ./... and don't use force-push to your PRs.)

Thank you!

[hemachandarv]

@gmlewis I saw that there was a closed PR (#1540) related to this issue. Can I take this up?

[gmlewis]

@gmlewis I saw that there was a closed PR (#1540) related to this issue. Can I take this up?

Thanks, @hemachandarv , but let's see how #625 goes before you also attempt to tackle this one.
We'll leave this one open so someone else can take it while you work on the other one.
Then, if this is still available when you are done with the other, we can assign it to you.

[hemachandarv]

Sounds like a plan. Thanks, @gmlewis

[hemachandarv]

@gmlewis As #625 is about to be closed, can I take this up?

[gmlewis]

@gmlewis As #625 is about to be closed, can I take this up?

Sure, @hemachandarv , it is yours. Thank you!

[amarlearning]

@gmlewis I believe good-first-issue should be kept for people who are contributing for the first time to this project.

Since there are not many good-first-issue, I believe such issues should be reserved for new contributors and someone who has already solved one good-first-issue should look for other issues.

Please correct me if I am wrong 😄

[gmlewis]

@amarlearning - I have no problem with any contributor to work on any issue in this repo. We are not reserving any issues for any categories of developers and really enjoy getting down to zero net issues. These labels are an attempt to be helpful but are not strict guidelines.

So please feel free to contribute to any issues that you feel you can resolve.

Thank you!