Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 ?
| return'cid:'.$image; | ||
| } | ||
| publicfunctionattach(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?
Uh oh!
There was an error while loading.Please reload this page.
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 to7cdbd20CompareI'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#17303Ref#17353Refsymfony/symfony#47711Refsymfony/symfony#47462This 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 changesThe title ofsymfony/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 insymfony/symfony#48060 whereas the use in the example code was correct.Commits-------46923d0 [Mailer] Fix attachment changes
#47462 follow-up