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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Helix for loading Emote Set info #175

Merged
merged 11 commits into from
Jul 3, 2021

Conversation

pajlada
Copy link
Member

@pajlada pajlada commented Jun 20, 2021

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

  • Create the *helix.Client in an internal package, making it passable to multiple packages that may want to use the same client.
  • Start replacing twitchemotes.com calls with Helix Get Emote Sets calls

Changes:

  • The /twitchemotes/sets/ endpoint was removed completely - it was never implemented nor used
  • The nicklaw/helix API Client is now initialized in cmd/api/main.go and passed through to any users. This allows multiple packages to share the same client.
  • We now create a "helix username" cache that can be used to resolve a Twitch ID to Twitch Username. Resolves are cached for 1h and are currently only used in the twitchemotes package
  • Removed any custom emote set modifications we previously did
  • For /twitchemotes/set/{setID}/ we only populate the bare minimum fields: channel_name and channel_id. The rest of the fields are still part of the JSON response, just left to their default values (empty string/0/false)
  • Twitchemotes route code moved to its own internal package under /internal/routes/
  • A trailing slash is no longer required for the /twitchemotes/set/{setID}/ endpoint

Closes #170
Closes #174

@pajlada pajlada force-pushed the fix/use-helix-for-getting-emote-set-info branch from 07ccf10 to f97b4ec Compare June 20, 2021 12:05
zneix
zneix previously requested changes Jun 20, 2021
internal/twitchapiclient/client.go Outdated Show resolved Hide resolved
internal/resolvers/twitch/resolver.go Show resolved Hide resolved
@zneix zneix added the blocked Something out of our power needs to change before proceeding with this issue label Jun 22, 2021
@zneix zneix removed the blocked Something out of our power needs to change before proceeding with this issue label Jun 28, 2021
@pajlada pajlada force-pushed the fix/use-helix-for-getting-emote-set-info branch from 80d8acb to 70bb12c Compare July 3, 2021 10:12
@pajlada pajlada requested a review from zneix July 3, 2021 10:52
@pajlada pajlada marked this pull request as ready for review July 3, 2021 11:11
@pajlada pajlada dismissed zneix’s stale review July 3, 2021 11:16

Talked this through already

Copy link
Collaborator

@zneix zneix left a comment

Choose a reason for hiding this comment

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

🚀

@pajlada pajlada force-pushed the fix/use-helix-for-getting-emote-set-info branch from a9a7095 to 81f4c1d Compare July 3, 2021 13:09
@zneix zneix enabled auto-merge (squash) July 3, 2021 13:38
@zneix zneix merged commit 1480f77 into master Jul 3, 2021
@zneix zneix deleted the fix/use-helix-for-getting-emote-set-info branch July 3, 2021 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants