Skip to content

Conversation

@Eseperio
Copy link

Current visibility of properties $email and $charset (private) limits the ability to serialize any inherited class, since __sleep(), when is called in a subclass, cannot access those properties and thus serialization is missing the most important part of class, the email. Changing $email and $charset to protected addresses this.

Q A
Is bugfix?
New feature?
Breaks BC?

Current visibility of properties $email and $charset (private) limits the ability to serialize any inherited class, since __sleep(), when is called in a subclass, cannot access those properties and thus serialization is missing the most important part of class, the email. 
Changing $email and $charset to protected addresses this.
@codecov
Copy link

codecov bot commented Oct 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.14%. Comparing base (d843854) to head (240ff21).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #82      +/-   ##
============================================
+ Coverage     85.07%   85.14%   +0.07%     
  Complexity       71       71              
============================================
  Files             4        4              
  Lines           201      202       +1     
============================================
+ Hits            171      172       +1     
  Misses           30       30              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@samdark samdark added this to the 3.0.1 milestone Oct 29, 2025
@samdark
Copy link
Member

samdark commented Oct 29, 2025

@SamMousa would you please take a look?

@SamMousa
Copy link
Contributor

  1. __sleep() is deprecated. https://wiki.php.net/rfc/soft-deprecate-sleep-wakeup
  2. I don't think you should be extending framework classes, it is an anti-pattern
  3. If you really need to serialize your dependent class I'd accept a PR that implements Serializable in the Message class and removes the __sleep() implementation, so that you could use parent::serialize() in your own class.
  4. Both the properties you mention have public getters...

Making stuff more open is usually not the way forward. I'd even argue that this class should be final.

@samdark
Copy link
Member

samdark commented Oct 29, 2025

Implementing Serializable is a very good idea.

@samdark samdark self-requested a review October 29, 2025 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants