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

[Mime] Simplify adding Parts to an Email#47462

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.2fromfabpot:mime-refactoring
Sep 28, 2022

Conversation

@fabpot
Copy link
Member

@fabpotfabpot commentedSep 2, 2022
edited
Loading

QA
Branch?6.2
Bug fix?no
New feature?yes
Deprecations?no
Ticketsn/a
LicenseMIT
Doc PR

While fixing some MIME bugs, I realized we have a substantial cyclomatic complexity due to the attach/attachFromPath/embed/embedFromPath/attachPart methods on the Email class.

This PR simplifies all of that and introduces a way to have a file for TextPart as well (via the newfile:// notation) and it keeps the lazy-loading feature which was why those methods were introduced in the first place.

From now, I've kept all the methods, but I'm wondering if we should deprecate all of them and only keepattachPart() (which I would like to renameaddPart()).

Kocal reacted with heart emoji
@carsonbot
Copy link

Hey!

I think@alamirault has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

Copy link
Contributor

@alamiraultalamirault left a comment

Choose a reason for hiding this comment

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

I think, deprecate methods in favor of one unique method is a good thing

@fabpotfabpotforce-pushed themime-refactoring branch 2 times, most recently from8a4b63b to0811e87CompareSeptember 25, 2022 16:57
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

I like it :)
Just some minor things.

@fabpotfabpot merged commit527e30f intosymfony:6.2Sep 28, 2022
@fabpotfabpot deleted the mime-refactoring branchSeptember 28, 2022 07:40
fabpot added a commit that referenced this pull requestOct 15, 2022
…l::addPart() (fabpot)This PR was merged into the 6.2 branch.Discussion----------[Mime] deprecate attach/embed methods in favor of Email::addPart()| Q             | A| ------------- | ---| Branch?       | 6.2| Bug fix?      | no| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | yes <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets       | n/a <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->| License       | MIT| Doc PR        | -#47462 follow-upCommits-------7cdbd20 [Mime] deprecate attach/embed methods in favor of Email::addPart()
javiereguiluz added a commit to javiereguiluz/symfony-docs that referenced this pull requestOct 20, 2022
…hiddewie)This PR was squashed before being merged into the 6.2 branch.Discussion----------Use `addPart` instead of `embed*` or `attach*`.Fixessymfony#17303Refsymfony#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*`.
@fabpotfabpot mentioned this pull requestOct 24, 2022
fabpot added a commit that referenced this pull requestSep 29, 2023
This PR was squashed before being merged into the 6.3 branch.Discussion----------[Mime] Fix email (de)serialization issues| Q             | A| ------------- | ---| Branch?       | 6.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Tickets       |Fix#47991| License       | MIT| Doc PR        | -#48156fixed#47991 while introducing a big breaking change (the `File` lazy load feature is broken and that was the whole point of that class when it was introduced in#47462 as that feature existed even prior to that PR) on a minor Symfony version (updating from 6.1 to 6.2 broke our application). More context can be found here:#48156 (comment)This PR aims to revert back the `attachFromPath` behavior to what it was before#48156 while still fixing the deserialization issue reported in#47991The first commit fixes the serialization logic to work the same way it had worked on both 5.4 and 6.1 (which means we are reverting#48156), while the second commit fixes the deserialization issue reported in#47991.I've also added tests to prevent serialization/deserialization regressions in the future.Commits-------32836b9 [Mime] Fix email (de)serialization issues
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@dunglasdunglasAwaiting requested review from dunglasdunglas is a code owner

+1 more reviewer

@alamiraultalamiraultalamirault requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

6.2

Development

Successfully merging this pull request may close these issues.

4 participants

@fabpot@carsonbot@nicolas-grekas@alamirault

[8]ページ先頭

©2009-2025 Movatter.jp