Skip to content
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

Provide serialized way. #397

Merged
merged 4 commits into from
Feb 7, 2024
Merged

Provide serialized way. #397

merged 4 commits into from
Feb 7, 2024

Conversation

dereuromark
Copy link
Owner

Resolves #396

@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (02dcd12) 72.53% compared to head (70ba172) 73.06%.
Report is 3 commits behind head on master.

❗ Current head 70ba172 differs from pull request most recent head 9e73a6e. Consider uploading reports for the commit 9e73a6e to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #397      +/-   ##
============================================
+ Coverage     72.53%   73.06%   +0.53%     
- Complexity      684      688       +4     
============================================
  Files            40       40              
  Lines          2450     2454       +4     
============================================
+ Hits           1777     1793      +16     
+ Misses          673      661      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@LordSimal
Copy link
Contributor

Well, if that message object contains a file then the contents of that file will also be serialized/base64 encoded as can be seen here

So this would then go against what we initially discussed where only the minimum necessary parts are being passed down to the queue/task and the task itself builds the object.

I am personally not sure what the correct way really is, since we mention sending emails without using a Mailer in the docs so it should still be viable... I just don't like it that way 😅

@dereuromark
Copy link
Owner Author

Yeah, it would mainly be to ease upgrading efforts maybe.
But in the end it would probably also become deprecated and removed, and one has to do this in their own custom class probably (respecting the size of the data field^^).

@dereuromark dereuromark merged commit 5a1f127 into master Feb 7, 2024
16 checks passed
@dereuromark dereuromark deleted the serialized branch February 7, 2024 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EmailTask with ['settings' => $this->getMessage()] not working any more
3 participants