-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Replace viper with koanf in builder #4757
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpkrohling are you ok with this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently, there's no change in the user-visible behavior, confirmed by comparing the output of go run ./ --help
. I'm fine with this change.
Codecov Report
@@ Coverage Diff @@
## main #4757 +/- ##
=======================================
Coverage 90.69% 90.69%
=======================================
Files 181 181
Lines 10616 10616
=======================================
Hits 9628 9628
Misses 772 772
Partials 216 216 Continue to review full report at Codecov.
|
The tests seem to be failing, I just sent in a PR fixing them: #4771. Once that one is merged, I'll rebase this. |
Please rebase :) Maybe see how to fix the tests. |
52a7b3b
to
9a8a40b
Compare
PR rebased |
There's something wrong: running diff --git a/cmd/otelcorecol/main.go b/cmd/otelcorecol/main.go
index 8f4124d6..b0885d17 100644
--- a/cmd/otelcorecol/main.go
+++ b/cmd/otelcorecol/main.go
@@ -19,9 +19,9 @@ func main() {
}
info := component.BuildInfo{
- Command: "otelcorecol",
- Description: "Local OpenTelemetry Collector binary, testing only.",
- Version: "0.43.1-dev",
+ Command: "otelcol-custom",
+ Description: "Custom OpenTelemetry Collector distribution",
+ Version: "1.0.0",
}
if err := run(service.CollectorSettings{BuildInfo: info, Factories: factories}); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm blocking this, as the generated resources seem to have been affected when they shouldn't.
@rleungx, are you still interested in this one? |
9a8a40b
to
766b1bf
Compare
766b1bf
to
0366b55
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make otelcorecol
, and the code looks sane. If the tests are passing as well, this is good to do.
812e125
to
2863718
Compare
Fixed, PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please resolve the conflicts, the change looks good otherwise
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
2863718
to
7281916
Compare
Description:
Just as the title says and also clean up the dependencies.
Link to tracking Issue: #4732
Testing: < Describe what testing was performed and which tests were added.>
Not sure if needed