Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate /commercial command to the Helix API #4094

Merged
merged 7 commits into from
Nov 5, 2022

Conversation

xel86
Copy link
Contributor

@xel86 xel86 commented Nov 1, 2022

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

Migrated the commercial command to the new helix API. Functionality is timegated so the command uses the IRC functionality as long as possible. I yoinked this PR from pajlada with his permission, approximately 57.8% of this PR's code is his 馃 !

Closes #3964

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/providers/twitch/api/Helix.cpp Show resolved Hide resolved
src/providers/twitch/api/Helix.cpp Show resolved Hide resolved
src/providers/twitch/api/Helix.cpp Show resolved Hide resolved
src/providers/twitch/api/Helix.cpp Show resolved Hide resolved
src/providers/twitch/api/Helix.cpp Show resolved Hide resolved
src/providers/twitch/api/Helix.hpp Outdated Show resolved Hide resolved
src/widgets/settingspages/GeneralPage.cpp Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

tests/src/HighlightController.cpp Show resolved Hide resolved
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just some nitpicks, other than that looks good and works as expected

if you would, please create a github issue for:

  1. retry_after missing from cooldowned requests
  2. error for invalid length contains the wrong message
    invalid length response:
{
  "data": [],
  "error": "Bad Request",
  "message": "To start a commercial, the broadcaster must be streaming live.",
  "status": 400
}

src/controllers/commands/CommandController.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

src/providers/twitch/api/Helix.cpp Show resolved Hide resolved
src/providers/twitch/api/Helix.cpp Show resolved Hide resolved
@xel86
Copy link
Contributor Author

xel86 commented Nov 3, 2022

Additionally, I opened a github issue for twitch to fix the issues mentioned: twitchdev/issues#686

@xel86 xel86 requested a review from pajlada November 4, 2022 13:55
@pajlada pajlada enabled auto-merge (squash) November 5, 2022 09:25
@pajlada pajlada merged commit f00f766 into Chatterino:master Nov 5, 2022
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.

Migrate /commercial command to Helix API
2 participants