Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Notifier] [Discord] Fix value limits#60269

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:6.4fromnorkunas:fix-discord-value-limits
Apr 27, 2025

Conversation

norkunas
Copy link
Contributor

QA
Branch?6.4
Bug fix?yes
New feature?no
Deprecations?no
IssuesN/A
LicenseMIT

It was not properly calculating values length with non latin characters for embed data. This makes it consistent with calculating chat message length inDiscordTransport.

@carsonbotcarsonbot added this to the6.4 milestoneApr 25, 2025
@OskarStarkOskarStark changed the title[Notifier] [Discord] Fix value limits[Notifier][Discord] Fix value limitsApr 25, 2025
@OskarStark
Copy link
Contributor

Hmm, what about removing the client side validation completely and let the API return a proper error? Can you check what is happening if we would do so?
My point is, that we cannot keep track of all new and updated limitations in the future and we always need a fix PR + a release 🤷‍♂️

@carsonbotcarsonbot changed the title[Notifier][Discord] Fix value limits[Notifier] [Discord] Fix value limitsApr 25, 2025
@norkunas
Copy link
ContributorAuthor

I'm pretty sure that limits were not changed, just wrong calculation was used :) I'm fine either way. But at least for me it's easier when i can click on the method likeaddField and see what are the limits, instead of going to discord docs.

Copy link
Contributor

@OskarStarkOskarStark left a comment

Choose a reason for hiding this comment

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

Ok then, lets do it

Copy link
Member

@fabpotfabpot left a comment

Choose a reason for hiding this comment

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

I'm going to merge it as a bug fix, but Oskar is right, we avoid doing the validation on our side as it forces us to make sure we update the code whenever upstream changes as well.

@fabpot
Copy link
Member

Thank you@norkunas.

norkunas reacted with thumbs up emoji

@fabpotfabpot merged commit06cccc5 intosymfony:6.4Apr 27, 2025
11 checks passed
@norkunasnorkunas deleted the fix-discord-value-limits branchApril 27, 2025 15:50
This was referencedMay 2, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@fabpotfabpotfabpot approved these changes

@OskarStarkOskarStarkOskarStark approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.

4 participants
@norkunas@OskarStark@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp