Skip to content

[Mime] Tweak an exception to be more descriptive #57295

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

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Jun 3, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? no
Deprecations? no
Issues Fix #54162
License MIT

@dakujem
Copy link

dakujem commented Jun 3, 2024

Fabien, I still suggest adding a code comment referencing the RFC 2392 (maybe also RFC 822) so that devs know why the restriction is in place. Maybe simple

// CID must contain a @ character to comply with RFC 2392 (RFC 822 syntax for "addr-spec")

above the exception would do.

@fabpot
Copy link
Member Author

fabpot commented Jun 3, 2024

Fabien, I still suggest adding a code comment referencing the RFC 2392 (maybe also RFC 822) so that devs know why the restriction is in place. Maybe simple

// CID must contain a @ character to comply with RFC 2392 (RFC 822 syntax for "addr-spec")

above the exception would do.

I've not added the reference to the spec as we're not doing it anywhere else in the code base. Having it only for this very specific error message would be weird.

@fabpot fabpot merged commit ea47cb4 into symfony:7.2 Jun 4, 2024
@fabpot fabpot deleted the better-exception branch June 4, 2024 06:35
@dakujem
Copy link

dakujem commented Jun 7, 2024

It may be a good time to start, you know 😉

In this case, it is exactly the reason to document - a very specific case and a restriction that does not seem to make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Mailer] DataPart#setContentId requires @
3 participants