Skip to content

[Mailer] Add additional headers in Scaleway bridge #57499

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 25, 2024
Merged

[Mailer] Add additional headers in Scaleway bridge #57499

merged 1 commit into from
Jun 25, 2024

Conversation

MrMicky-FR
Copy link
Contributor

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
License MIT

This adds the missing additional headers in the Scaleway bridge (I forgot to add them in the original implementation). This can be used, for example, to add a Reply-To header that doesn't have its own field, but is supported in the additional_headers field (see the Scaleway documentation). Tested with the Scaleway API and is working fine 🚀

@OskarStark
Copy link
Contributor

This is a new feature and must target 7.2 branch please

@MrMicky-FR
Copy link
Contributor Author

MrMicky-FR commented Jun 23, 2024

Thanks for your quick answer, but isn't this more of a bug fix, since Email methods like replyTo or priority are currently ignored with this bridge, when they shouldn't be?

@OskarStark
Copy link
Contributor

One could discuss, as the bridge ignores them now, it is some kind of behavior change and we are always very careful with merging such things.
From my side a new feature, but open for a bugfix. Let's see what other members say 😎

Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Works for me as a bugfix. Or as a feature if we wanna be strict, but for bridges I'm tempted to be more relaxed. Dunno what others think :)

@fabpot
Copy link
Member

fabpot commented Jun 25, 2024

Without a doubt, this is a new feature (as it introduces a new behavior, even the PR title makes it very clear: "Add...").
But as the Reply-To header is important, I'm going to merge it as a bug fix.

@fabpot
Copy link
Member

fabpot commented Jun 25, 2024

Thank you @MrMicky-FR.

@fabpot fabpot merged commit bafe768 into symfony:6.4 Jun 25, 2024
8 of 10 checks passed
@MrMicky-FR MrMicky-FR deleted the scaleway_mailer_headers branch June 25, 2024 06:44
This was referenced Jun 28, 2024
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.

5 participants