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

additional mailbox selector for typed props #1096

Merged
merged 13 commits into from
Aug 28, 2024

Conversation

Roiocam
Copy link
Member

@Roiocam Roiocam commented Feb 3, 2024

Motivation

I have experience in migrating Classic to Typed. In the process, I found that Typed can't customize Mailbox through Props. We should match the same Classic method on Typed.

截屏2024-02-03 17 03 39

TODO list

  • Implementation: Reuse existing classes
  • Mention in documentation
  • Verify via unit test

@Roiocam Roiocam marked this pull request as draft February 3, 2024 09:16
@pjfanning
Copy link
Contributor

@Roiocam can you put the typo fixes in a separate PR? We can backfit that change.

@Roiocam Roiocam mentioned this pull request Feb 5, 2024
parentProbe.ref)

val childProbe = createTestProbe[ActorRef[Ping]]()
grandParent ! SpawnProtocol.Spawn(PingPong(), "child", Props.empty.withDispatcherSameAsParent, childProbe.ref)
Copy link
Member Author

Choose a reason for hiding this comment

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

Fix children not spawn from the parent

@Roiocam Roiocam force-pushed the additional-typed-props-mailbox-selector branch from 3b83298 to 8e3f603 Compare February 6, 2024 06:16
@Roiocam Roiocam force-pushed the additional-typed-props-mailbox-selector branch from 93fac01 to b9b046d Compare February 6, 2024 07:23
@Roiocam Roiocam force-pushed the additional-typed-props-mailbox-selector branch from d1302b7 to 7cbc958 Compare February 6, 2024 08:19
@Roiocam Roiocam marked this pull request as ready for review February 6, 2024 08:20
@Roiocam Roiocam force-pushed the additional-typed-props-mailbox-selector branch from 7cbc958 to 9c1022f Compare February 6, 2024 08:23
@Roiocam Roiocam requested a review from pjfanning April 26, 2024 06:12
import org.apache.pekko
import pekko.actor.typed.DispatcherSelector
Copy link
Contributor

Choose a reason for hiding this comment

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

can you reorder the imports and put back the empty line above?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry about the delay, could you review this again and contain this in release 1.1.0?

@pjfanning pjfanning added this to the 1.1.0 milestone Aug 23, 2024
Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

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

lgtm - but it would be good if someone else reviews this too

@pjfanning
Copy link
Contributor

@mdedetrich would you have time to review this? I'm hoping to get the RC for v1.1.0 together in the next few days.

@mdedetrich
Copy link
Contributor

I will look at it tonight

@pjfanning
Copy link
Contributor

@kerr would you have time to review this?

Copy link
Contributor

@mdedetrich mdedetrich left a comment

Choose a reason for hiding this comment

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

lgtm although would really like a review from someone else. If all else fails we can use @ApiMayChange as a clutch.

@pjfanning pjfanning merged commit d1ec224 into apache:main Aug 28, 2024
19 checks passed
@pjfanning
Copy link
Contributor

I looked again and I think this is pretty safe.

@Roiocam Roiocam deleted the additional-typed-props-mailbox-selector branch August 29, 2024 01:30
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.

4 participants