-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Mime] deprecate attach/embed methods in favor of Email::addPart() #47711
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
Conversation
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.
Changelog entries missing :)
But I'm wondering: is it worth it? I mean: this will have an impact on all current users of the component, and will make it harder for bundles to provide compat with many versions at once (and be deprecation-free). Also, it's a bit longer to type and might be a bit harder to discover (embed()
vs ->asInline()
). What's the issue with the current way of doing (and naming ;)) things ?
$this->message->embed($file->getCode(), $image, $contentType); | ||
} | ||
$body = $file->getPath() ? new BodyFile($file->getPath()) : $file->getCode(); | ||
$this->message->addPart((new DataPart($body, $image, $contentType))->asInline()); | ||
|
||
return 'cid:'.$image; | ||
} | ||
|
||
public function attach(string $file, string $name = null, string $contentType = null): void |
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.
should be deprecated also I guess?
As mentioned in the related PR, I'm not sure this is worth it either. That's just a proposal. |
To make it easier to support multiple versions, we might keep those names around for now, and deprecate them only in 7.1. As those methods are implemented as shortcuts on top of the new API, their maintenance cost is quite low. However, if you expect to rename |
Thanks for the insights. I'm "-0" personnaly: not worth the trouble pushed to the community IMHO. |
I would at least keep the removal of using these methods internally and I would advocate for adding the addPart() method to replace the attachPart one. |
0530036
to
7cdbd20
Compare
I've removed all deprecations but the one for |
This PR was squashed before being merged into the 6.2 branch. Discussion ---------- Use `addPart` instead of `embed*` or `attach*`. Fixes #17303 Ref #17353 Ref symfony/symfony#47711 Ref symfony/symfony#47462 This PR contains the documentation for simplifications for adding parts to `Email`s. Commits ------- ebbffc5 Use `addPart` instead of `embed*` or `attach*`.
This PR was squashed before being merged into the 6.3 branch. Discussion ---------- [Mailer] Fix attachment changes The title of symfony/symfony#47711 did not change before merge, but actually only `attachPart` has been deprecated (renamed) in favor of `addPart`. I did not reintroduce the other methods as they may be deprecated in 7.1, so no need to document a way that may change in a few months. Also the `BodyFile` class has been renamed in symfony/symfony#48060 whereas the use in the example code was correct. Commits ------- 46923d0 [Mailer] Fix attachment changes
#47462 follow-up