nicklaw5 / twitch-api-php

A Twitch API client for PHP.
https://packagist.org/packages/nicklaw5/twitch-api-php
MIT License
116 stars 48 forks source link

Add support for the "helix" API #11

Closed nicklaw5 closed 5 years ago

nicklaw5 commented 6 years ago

See https://dev.twitch.tv/docs/api/reference

Helix integration should include support for webhooks (see #13).

nicklaw5 commented 6 years ago

Inadvertently closed

cihantas commented 6 years ago

Is Helix support still desired for PHP or are you only maintaining the Golang version from now on?

echosa commented 6 years ago

I certainly hope that PHP will be updated as well! Our site definitely isn't switching to Golang anytime soon, and with the old Twitch APIs being retired at the end of the year, this package will definitely need to be updated.

nicklaw5 commented 6 years ago

At present, I do not have the time to contribute to the task of catering this library to the new Helix API. Hence, why I've added the help wanted tag to this issue.

I am very happy to accept PRs with regard to adding support for the Helix API, but I simply can't take on this task myself at this time.

echosa commented 6 years ago

I think I said it before, but just in case I'll say it again: I'll try to help tackle this when I can. Hopefully sooner rather than later!

echosa commented 5 years ago

@nicklaw5 FYI I have been working on adding Helix support. I forked this repo and created a new branch. Link at the bottom of this comment.

What I've decided to do is create a new package (NewApi) within the src directory to hold all the new Helix code and endpoints. This allows the existing Kraken API code you've written to remain silo-ed and untouched, which provides two benefits: 1) the existing working code isn't broken, and 2.) when it comes time to drop Kraken support, it's easy to just delete everything but the new Helix code.

I'm also changing up how the codebase looks and and is organized, within the new package. I personally find the current code a bit muddled and hard to follow, with the use of traits, etc. I'm trying to make the new code easier to follow and maintain, using more direct dependency injection and such, which essentially results in the new code being a completely separate effort from the existing code (which also plays into my two points in the last paragraph).

A final point I want to make is that I'm starting with just the API calls that I need for my own purposes right now, which is essentially limited to just oauth, getting user info, getting stream info, and stream up/down webhooks. These are the API endpoints I need, so I'm getting those working first. Everything else is secondary to me.

The Kraken API is deprecated at the end of this year, so I'm trying to at least get the new API endpoints that I need done well before then. After than, I can spend some time getting other endpoints working, though, like you, I'll likely look for help. If you're still busy, that's fine. If you have time and are willing to work within this new package and basically new code base I've started on, I welcome the help. :-)

I'd love your thoughts, since I'm far enough in that the concepts and initial work is done but not so far in that everything is necessarily set in stone. You'll see that I added a new command-line tool for making API calls from the command-line, essentially providing a built-in CLI client for the new API endpoints, which allows for easy manual testing. This has been indispensably helpful during my efforts. Also, if there's a way to assign me this ticket, feel free, since I'm working on it anyway, and that will be good information for anyone coming to this ticket for answers.

You can see the work-in-progress branch diffed against master here: https://github.com/nicklaw5/twitch-api-php/compare/master...echosa:new-twitch-api?expand=1

nicklaw5 commented 5 years ago

I personally find the current code a bit muddled and hard to follow, with the use of traits, etc

I couldn't agree more. I 100% regret using traits. It's not the "right" way to write PHP in the context of an API client.

The Kraken API is deprecated at the end of this year

I noticed a few months ago that Twitch ended removing the removal date of the v5 API. I can only assume there were a lot of people on the forums complaining that Helix was nowhere near finished and deprecating it v5 in December would cause more harm than good.

image

You'll see that I added a new command-line tool for making API calls from the command-line

Great idea!


I can't assign you to the issue unless you're a maintainer of this library. But if you want to go ahead and create a PR, I can start reviewing the code.

From a cursory glance it looks mostly fine. I would like to see some tests, however. That's one thing I never got around to doing thoroughly from my original implementation. As an example, you can see how I structured the tests in nicklaw5/helix. Note that the API calls are mocked rather than sending actual HTTP requests.

I'm moving interstate next week and start a new position a week after that, so I won't have a lot of time to help in the coming weeks, but I'll try to do what I can when I have some time.

echosa commented 5 years ago

When I think it's ready, I'll definitely send in a PR. :-) I do want to try to get some tests in place. I'll have to mock out the actual API calls out. The Twitch documentation gives examples of responses, so I can just use those. That's quite helpful!

I'm moving interstate next week and start a new position a week after that

Sounds like congrats are in order! 🎉

echosa commented 5 years ago

Just an update: still working on this. Haven't abandoned it. Just taking longer than expected! Doing what I can ASAP, though.

echosa commented 5 years ago

Huge update: I believe I have completed the work necessary for the Helix upgrade (again, limited to the API calls I personally use for now). I've moved my work into a new forked repo under my company's account, and have created a PR for them to look over an review. (It's good to have a separate set of eyes, especially since this will be part of our own site.)

After everything is all good internally at my company and the PR is approved there, I'll create an upstream PR from there to here. We're so close… Soon! :-D

You can see the changes (as of now) ahead of time by looking my own personal fork's branch. I say "as of now", because, henceforth, all changes I make will be in our company repo. Those will be available in the final upstream PR, but if you want to get a head start on looking over the changes, here you go:

https://github.com/nicklaw5/twitch-api-php/compare/master...echosa:new-twitch-api?expand=1

nicklaw5 commented 5 years ago

Sounds absolutely fantastic! Can't wait!

echosa commented 5 years ago

Initial Helix support has been added. I'm closing this ticket in favor of https://github.com/nicklaw5/twitch-api-php/issues/24 for continuing development.