-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
feat(structures): add SoundboardSound structure #11390
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
feat(structures): add SoundboardSound structure #11390
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11390 +/- ##
==========================================
- Coverage 31.80% 31.75% -0.06%
==========================================
Files 380 381 +1
Lines 13838 13860 +22
Branches 1093 1095 +2
==========================================
Hits 4401 4401
- Misses 9303 9325 +22
Partials 134 134
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughThis pull request introduces a new soundboards module to the structures package by creating a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/structures/src/soundboards/SoundboardSound.ts`:
- Around line 22-27: Update the JSDoc on the SoundboardSound constructor: change
the `@param` description that currently says "The raw data received from the API
for the emoji" to "The raw data received from the API for the soundboard sound"
(parameter is the constructor public constructor(data:
Partialize<APISoundboardSound, Omitted>) in class SoundboardSound) so the doc
correctly describes the data type and intent.
- Around line 78-95: The guild check in the createdTimestamp getter is overly
verbose and casts unnecessarily; update createdTimestamp (accessing
this[kData].sound_id and this[kData].guild_id) to determine inGuild with a
simple Boolean(this[kData].guild_id) and call DiscordSnowflake.timestampFrom
with the sound_id directly (remove the unnecessary "as string" cast), leaving
createdAt to continue returning new Date(timestamp) or null as before.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
packages/structures/src/index.tspackages/structures/src/soundboards/SoundboardSound.tspackages/structures/src/soundboards/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Tests
🔇 Additional comments (4)
packages/structures/src/soundboards/SoundboardSound.ts (2)
1-6: LGTM!Imports are appropriate - using
@sapphire/snowflakefor timestamp extraction and proper types fromdiscord-api-types/v10.
29-76: LGTM!The getters correctly map API fields to properties following the established pattern in this codebase. The naming conventions (camelCase for JS, snake_case access to raw data) are consistent.
packages/structures/src/soundboards/index.ts (1)
1-1: LGTM!Standard barrel export following the established pattern in this codebase.
packages/structures/src/index.ts (1)
9-9: LGTM!The soundboards module export is correctly placed in alphabetical order and follows the established barrel export pattern.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
This PR will add the
SoundboardSoundAPI structure, as documented on the Discord API documentation here.This PR does not add tests for this structure.
Thanks to @edocsil47 for helping on how to determine if a guild id is present within the structure.
Mentioning #10981 for visibility.