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.

Sign up for GitHub

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

Jump to bottom

Add change handler to register callbacks #1292

Merged
merged 3 commits into from Dec 1, 2022
Merged

Add change handler to register callbacks #1292

merged 3 commits into from Dec 1, 2022

Conversation

frzifus
Copy link
Member

@frzifus frzifus commented Nov 30, 2022 •
edited

Description: #1206 (comment)

@frzifus frzifus mentioned this pull request Nov 30, 2022
Merged
@frzifus frzifus force-pushed the add_change_handler_to_register_callbacks branch 2 times, most recently from 3031879 to 444f330 Compare November 30, 2022 15:26
@frzifus frzifus marked this pull request as ready for review November 30, 2022 15:28
@frzifus frzifus requested a review from a team as a code owner November 30, 2022 15:28
kevinearls
kevinearls approved these changes Nov 30, 2022
View reviewed changes
Copy link
Member

@kevinearls kevinearls left a comment

Choose a reason for hiding this comment

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

LGTM except for some minor issues on comments

internal/config/change_handler.go Outdated Show resolved Hide resolved
internal/config/change_handler.go Outdated Show resolved Hide resolved
internal/config/main.go Outdated Show resolved Hide resolved
@frzifus frzifus force-pushed the add_change_handler_to_register_callbacks branch from 444f330 to 1215eb8 Compare November 30, 2022 15:47
@frzifus frzifus force-pushed the add_change_handler_to_register_callbacks branch from 1215eb8 to eaff211 Compare November 30, 2022 17:43
pavolloffay
pavolloffay reviewed Dec 1, 2022
View reviewed changes
internal/config/change_handler.go Outdated Show resolved Hide resolved
internal/config/options.go Outdated Show resolved Hide resolved
pavolloffay
pavolloffay reviewed Dec 1, 2022
View reviewed changes
internal/config/main.go Outdated Show resolved Hide resolved
@frzifus
config: move onChange callback list methods into a thread-safe wrapper
3623079 
Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>
@frzifus
config: keep change handler private
c284c92 
Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>
@frzifus frzifus force-pushed the add_change_handler_to_register_callbacks branch from 88ae5ff to c96265c Compare December 1, 2022 16:52
@frzifus
follow naming recommendations
6a0e657 
Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>
@frzifus frzifus force-pushed the add_change_handler_to_register_callbacks branch from c96265c to 6a0e657 Compare December 1, 2022 16:54
@pavolloffay pavolloffay merged commit eb51a09 into open-telemetry:main Dec 1, 2022
@frzifus frzifus deleted the add_change_handler_to_register_callbacks branch December 1, 2022 17:29
ihalaij1 pushed a commit to ihalaij1/opentelemetry-operator that referenced this pull request Dec 8, 2022
@frzifus @ihalaij1
Add change handler to register callbacks (open-telemetry#1292)
e0cf059 
* config: move onChange callback list methods into a thread-safe wrapper

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>

* config: keep change handler private

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>

* follow naming recommendations

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
@frzifus
Add change handler to register callbacks (open-telemetry#1292)
a66f2aa 
* config: move onChange callback list methods into a thread-safe wrapper

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>

* config: keep change handler private

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>

* follow naming recommendations

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevinearls kevinearls kevinearls approved these changes

@iblancasa iblancasa iblancasa approved these changes

@pavolloffay pavolloffay pavolloffay approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@frzifus @kevinearls @iblancasa @pavolloffay