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

Deprecate confmap.Conf.Set, not used anywhere for the moment #5485

Merged
merged 1 commit into from
Jun 7, 2022

Conversation

bogdandrutu
Copy link
Member

@bogdandrutu bogdandrutu commented Jun 6, 2022

Fixes #4467

Signed-off-by: Bogdan Drutu bogdandrutu@gmail.com

@bogdandrutu bogdandrutu requested review from a team and Aneurysm9 June 6, 2022 18:12
@bogdandrutu
Copy link
Member Author

bogdandrutu commented Jun 6, 2022

cc @mx-psi can you tell me about your usage in DD?

I will "deprecate" it, but initially I am trying to see if contrib breaks. Update: contrib passed, so change this to deprecated.

@codecov
Copy link

codecov bot commented Jun 6, 2022

Codecov Report

Merging #5485 (6da9627) into main (0347b25) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5485      +/-   ##
==========================================
- Coverage   90.84%   90.79%   -0.06%     
==========================================
  Files         193      193              
  Lines       11430    11431       +1     
==========================================
- Hits        10384    10379       -5     
- Misses        824      830       +6     
  Partials      222      222              
Impacted Files Coverage Δ
confmap/confmap.go 84.15% <ø> (-5.95%) ⬇️
config/internal/configsource/manager.go 85.95% <100.00%> (ø)
confmap/converter/expandconverter/expand.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0347b25...6da9627. Read the comment docs.

@bogdandrutu bogdandrutu changed the title Remove confmap.Conf.Set, not used anywhere for the moment Deprecate confmap.Conf.Set, not used anywhere for the moment Jun 6, 2022
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
@mx-psi
Copy link
Member

mx-psi commented Jun 7, 2022

cc @mx-psi can you tell me about your usage in DD?

I will "deprecate" it, but initially I am trying to see if contrib breaks. Update: contrib passed, so change this to deprecated.

We use it to build up a confmap.Conf manually from Viper (see here). It's feasible to replace it with a Merge or handle it with something else, I just trusted this code more than using Viper 😄

@mx-psi
Copy link
Member

mx-psi commented Jun 7, 2022

DataDog/datadog-agent/pull/12296 removes all usages of Set on our codebase

@bogdandrutu bogdandrutu merged commit 0bc2777 into open-telemetry:main Jun 7, 2022
@bogdandrutu bogdandrutu deleted the rmset branch October 14, 2024 20:28
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.

Return an error on config.Map.Set
2 participants