Skip to content

chore: clean code in ClusterSharding#1305

Merged
Roiocam merged 2 commits intoapache:mainfrom
Roiocam:clean-code
May 19, 2024
Merged

chore: clean code in ClusterSharding#1305
Roiocam merged 2 commits intoapache:mainfrom
Roiocam:clean-code

Conversation

@Roiocam
Copy link
Member

@Roiocam Roiocam commented Apr 30, 2024

Make code cleaner, it is basically the same.

Before using

object PropsAdapter {
def apply[T](behavior: => Behavior[T], deploy: Props = Props.empty): pekko.actor.Props =
pekko.actor.typed.internal.adapter.PropsAdapter(
() => Behaviors.supervise(behavior).onFailure(SupervisorStrategy.stop),
deploy,
rethrowTypedFailure = false)

After

override def systemActorOf[U](behavior: Behavior[U], name: String, props: Props): ActorRef[U] = {
val ref = system.systemActorOf(
PropsAdapter(
() => Behaviors.supervise(behavior).onFailure(SupervisorStrategy.stop),
props,
rethrowTypedFailure = false),
name)
ActorRefAdapter(ref)
}

// using classic.systemActorOf to avoid the Future[ActorRef]
system.toClassic
.asInstanceOf[ExtendedActorSystem]
.systemActorOf(
Copy link
Member Author

Choose a reason for hiding this comment

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

The before code didn't return ActorRef with typed? looks like a bug too.

Copy link
Member

@He-Pin He-Pin left a comment

Choose a reason for hiding this comment

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

Lgtm

Copy link
Member

@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.

No merges should be happening at the moment. We have an open mail thread about doing a release. It is chaos if people keep merging changes. All changes - however small - should be discussed on the mail thread about the release.

@pjfanning pjfanning added this to the 1.1.0-M2 milestone May 7, 2024
@pjfanning pjfanning dismissed their stale review May 15, 2024 11:46

1.1.0-M1 release approved

Copy link
Member

@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

@Roiocam Roiocam merged commit 5d23adb into apache:main May 19, 2024
@Roiocam Roiocam deleted the clean-code branch May 19, 2024 13:48
@pjfanning pjfanning added the release notes Need to release note label Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes Need to release note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants