Skip to content

Conversation

Awakened-Redstone
Copy link

https://dev.twitch.tv/docs/api/reference/#get-eventsub-subscriptions specifies status, type, user_id, subscription_id and after as valid query parameters, but the GetEventSubSubscriptionsAsync method only had status, type, user_id and after

https://dev.twitch.tv/docs/api/reference/#get-eventsub-subscriptions specifies `status`, `type`, `user_id`, `subscription_id` and `after` as valid query parameters, but the GetEventSubSubscriptionsAsync method only had `status`, `type`, `user_id` and `after`
@MrLawbreaker
Copy link

MrLawbreaker commented Sep 19, 2025

This will be a breaking change for existing calls. Had similar with #417 with a new parameter.

Maybe this should also be a new method with a new Request class as parameter and marking this old one as obsolote?

@Awakened-Redstone
Copy link
Author

Awakened-Redstone commented Sep 20, 2025

Maybe this should also be a new method with a new Request class as parameter and marking this old one as obsolote?

That would work.
I'm unsure as on how C# will treat it (as I'm new to C#, specially), but another option I see is to have a 6 parameter (with the same structure as the old method) and a 7 parameter one, marking the 6 parameter one as obsolete.

I'll approach the preferred method by the TwitchLib team.

@AoshiW
Copy link

AoshiW commented Sep 20, 2025

another option I see is to have a 6 parameter (with the same structure as the old method) and a 7 parameter one

This is a bad idea. The original method has a default value set for all parameters (and all parameters are of type string), so this would cause problems.

So either I would merge it as it is or create a new method that will receive the "request" class.

Note: I looked through all the "Reques" classes, and they all represent the "body" of the request. So perhaps it would be better to use a slightly different suffix to distinguish between the body and the query. But maybe I'm overthinking it.

It would be better if someone from the TwitchLib team who is working on this project could tell his opinion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants