-
Notifications
You must be signed in to change notification settings - Fork 422
fix(firestore-send-email) restore headers support #2440
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
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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.
Pull Request Overview
This PR restores support for custom email headers in the Firestore email sending extension by reintroducing the missing headers field and updating documentation accordingly.
- Restores the headers property in the mail options in index.ts.
- Bumps the extension version to 0.2.4 in extension.yaml.
- Updates documentation and changelog to reflect this new feature.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| firestore-send-email/functions/src/index.ts | Adds the headers field to email payload to support custom headers. |
| firestore-send-email/extension.yaml | Updates the extension version. |
| firestore-send-email/README.md | Adds a section on using custom headers with example JSON. |
| firestore-send-email/CHANGELOG.md | Documents the addition of the headers field and updates the version. |
|
Hi, can you run |
|
I ran it but it removes the blurb I wrote about using Custom Headers. Where should I place that instead? PREINSTALL.md ? Edit: Done |
|
Any blockers on this PR? |
|
Hi! No blockers, just finding time to prioritise it. Sorry for the wait. |
|
Thanks, no problem. Just wondering if there was something else missing on my end. |
CorieW
left a comment
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.
|
Thank you! I'll work on these tomorrow morning! |
|
@CorieW If I did things right, it should be mergeable now. I used version 0.2.5 as it was the next number in the list. |
|
Hi! lgtm, i'll try and get this released this week. |
|
Closing to make some minor adjustments before release in #2463 Thank you for your PR! |
|
Super discouraging, but alright. :-/ |
|
Apologies @Nushio, this was miscommunication on my part. I'll see what I can do about ensuring some attribution is recognised |
|
Thank you! |
|
Sorry about this, @Nushio I hope this experience doesn't discourage you. We really appreciate your contributions and want to support and encourage all contributors. Once again, thank you for your work on this! |


I just noticed that this feature was removed, and wanted to contribute a quick fix.