Skip to content

Conversation

@Kybxd
Copy link
Collaborator

@Kybxd Kybxd commented Jul 16, 2025

No description provided.

@Kybxd Kybxd marked this pull request as draft July 16, 2025 09:32
@Kybxd Kybxd force-pushed the go-messager-container-provider branch from cd647b2 to c62681c Compare July 17, 2025 03:00
@Kybxd Kybxd force-pushed the go-messager-container-provider branch from e9139c1 to d173be5 Compare September 10, 2025 09:12
@Kybxd Kybxd force-pushed the go-messager-container-provider branch from 3792631 to 0b02b55 Compare September 11, 2025 07:37
fmt.Printf("PatchReplaceConf: %v\n", hub.GetHub().GetPatchReplaceConf().Data())
// print provided patch replace conf
fmt.Printf("PatchReplaceConf(provided): %v\n", hub.GetHub().ProvidedBy(ctx).GetPatchReplaceConf().Data())
// print patch replace conf binding to background context
Copy link
Member

@wenchy wenchy Sep 11, 2025

Choose a reason for hiding this comment

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

Please obey the context API convention amd design elegant APIs:

  • ctx := hub.GetHub().BindTo(ctx) -> ctx = hub.NewContext(ctx, myhub)
  • myhub := hub.GetHub().ProvidedBy(ctx) -> myhub = hub.FromContext(ctx)

Refer to https://pkg.go.dev/context#Context
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We do not bind a hub to context, but bind its messager container. ctx = hub.NewContext(ctx, myhub) will lead to misunderstanding.

@@ -0,0 +1,208 @@
import (
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to add another abstraction: base hub.

"Keep it simple, stupid!"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed

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.

2 participants