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

Remove CloseMsgChan() method from collector process #388

Conversation

antoninbas
Copy link
Member

Having a seprate method to close an "output" channel is an anti-pattern IMO, and it is not clear how library consumers are supposed to use it, in particular in relation to the Stop() method.

Instead, we can close the channel when the Stop() method is closed, and all goroutines which may write to the channel have been stopped. Goroutines which read from the channel can use this as a signal to stop - or can be signalled directly by the goroutine which called Stop() in the first place.

Having a seprate method to close an "output" channel is an anti-pattern
IMO, and it is not clear how library consumers are supposed to use it,
in particular in relation to the Stop() method.

Instead, we can close the channel when the Stop() method is closed, and
all goroutines which may write to the channel have been
stopped. Goroutines which read from the channel can use this as a signal
to stop - or can be signalled directly by the goroutine which called
Stop() in the first place.

Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Copy link
Contributor

@yuntanghsu yuntanghsu left a comment

Choose a reason for hiding this comment

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

LGTM

@antoninbas antoninbas merged commit 75b0231 into vmware:main Dec 17, 2024
8 checks passed
@antoninbas antoninbas deleted the remove-CloseMsgChan-method-from-collector-process branch December 17, 2024 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants