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

Simplify envelopes #217

Closed
wants to merge 20 commits into from
Closed

Simplify envelopes #217

wants to merge 20 commits into from

Conversation

viktorprogger
Copy link
Contributor

Q A
Is bugfix?
New feature?
Breaks BC?

This PR simplifies the Envelope concept. The main changes are:

  • EnvelopeTrait is converted into AbstractEnvelope class.
  • If an envelope uses the new abstract class, there is no more envelope stack, which requires recursive data search. There is always a single envelope around a message.

With these changes it's dimpler to debug an application with envelopes. It's also becomes simple and meaningful to use unique method of envelopes be envelope creation in place like this, because there is no more a deep envelope stack with a recursive data search:

$id = IdEnvelope::fromMessage($message)->getId();

@viktorprogger viktorprogger self-assigned this Sep 20, 2024
@viktorprogger viktorprogger requested review from xepozz, samdark and a team September 20, 2024 13:48
@viktorprogger viktorprogger added the status:code review The pull request needs review. label Sep 20, 2024
.github/workflows/bechmark.yml Outdated Show resolved Hide resolved
src/Message/JsonMessageSerializer.php Outdated Show resolved Hide resolved
src/Message/Message.php Outdated Show resolved Hide resolved
@yiisoft yiisoft deleted a comment from samdark Nov 10, 2024
@viktorprogger
Copy link
Contributor Author

Trait to abstract class conversion is moved to #231, other changes slow down this package too much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:code review The pull request needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants