fix: message builders #10802
fix: message builders #10802
Conversation
almeidx
requested review from
vladfrangu,
iCrawl,
kyranet,
didinele and
Jiralite
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Skipped Deployments
|
- Added `clearParse`, `clearRoles`, and `clearUsers` methods to the `AllowedMentionsBuilder`, since passing an empty array and omitting the these fields behave differently - Strictened assertions - Removed `AttachmentBuilder#clearId`, as it is a required field - Added missing `MessageBuilder#setEmbeds` - Added missing `MessageReferenceBuilder#setFailIfNotExists` - Improve/fix documentation - Consistency:tm:
almeidx
force-pushed
the
fix/message-builders
branch
from
1cdd834 to
ec66fee
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10802 +/- ##
==========================================
+ Coverage 39.66% 39.90% +0.24%
==========================================
Files 249 249
Lines 15206 15239 +33
Branches 1456 1463 +7
==========================================
+ Hits 6031 6081 +50
+ Misses 9163 9146 -17
Partials 12 12
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:
|
| * @param embeds - The embeds to set | ||
| */ | ||
| public setEmbeds(...embeds: RestOrArray<APIEmbed | EmbedBuilder | ((builder: EmbedBuilder) => EmbedBuilder)>): this { | ||
| return this.spliceEmbeds(0, this.embeds.length, ...normalizeArray(embeds)); |
There was a problem hiding this comment.
I tested it and while it works, I would much rather stick to explicitly overriding the array instead on relying on splicing to replicate a set operator
There was a problem hiding this comment.
This (and the other changes like this) fall under the "Consistency:tm:" part of the description
There was a problem hiding this comment.
Well I am against relying on splice to do a set 🤷, would love to hear other's thoughts.
There was a problem hiding this comment.
Well I am against relying on splice to do a set 🤷, would love to hear other's thoughts.
I don't see a problem with using the splice method, but if we're not going to, then we should update it everywhere else too.
| * @param embeds - The embeds to set | ||
| */ | ||
| public setEmbeds(...embeds: RestOrArray<APIEmbed | EmbedBuilder | ((builder: EmbedBuilder) => EmbedBuilder)>): this { | ||
| return this.spliceEmbeds(0, this.embeds.length, ...normalizeArray(embeds)); |
There was a problem hiding this comment.
Well I am against relying on splice to do a set 🤷, would love to hear other's thoughts.
I don't see a problem with using the splice method, but if we're not going to, then we should update it everywhere else too.
clearParse,clearRoles, andclearUsersmethods to theAllowedMentionsBuilder, since passing an empty array and omitting the these fields behave differentlyAttachmentBuilder#clearId, as it is a required fieldMessageBuilder#setEmbedsMessageReferenceBuilder#setFailIfNotExists