-
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
Add local module replacement to ocb template #11653
Add local module replacement to ocb template #11653
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11653 +/- ##
==========================================
+ Coverage 91.58% 91.61% +0.02%
==========================================
Files 440 440
Lines 23768 23746 -22
==========================================
- Hits 21769 21755 -14
+ Misses 1627 1620 -7
+ Partials 372 371 -1 ☔ View full report in Codecov by Sentry. |
4066e12
to
fcf78ba
Compare
for some reason ad8ab82 is still pending for removal even though I force pushed. |
if the commit doesn't disappear today I'll close this PR and open a new one with a fresh branch. opened a ticket with GitHub because this is weird behavior |
ok I just re-added and then reverted the commit, seems to be fine now |
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.
LGTM
{{- range .Providers}} | ||
{{if ne .Path ""}}replace {{.GoMod}} => {{.Path}}{{end}} | ||
{{- end}} | ||
{{- range .Converters}} | ||
{{if ne .Path ""}}replace {{.GoMod}} => {{.Path}}{{end}} | ||
{{- end}} |
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.
Can you do a separate PR and rename .Providers -> .ConfmapProviders same for converter?
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.
yeah that makes sense to me
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.
do you want to wait to merge this one in until that PR is open? seems like it would make sense to merge this and then I can create a new branch from main with this included
…ers (#11678) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description change name in templates and cmd/builder to refer to Providers and Converters as ConfmapProviders and ConfmapConverters <!-- Issue number if applicable --> #### Link to tracking issue as requested here: #11653 (comment) <!--Describe what testing was performed and which tests were added.--> #### Testing existing tests should cover this <!--Describe the documentation added.--> #### Documentation none, internal change and no new code/api <!--Please delete paragraphs that you did not use before submitting.-->
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Adds ability to replace Providers and Converters with local paths when building custom collector in ocb <!-- Issue number if applicable --> #### Link to tracking issue Closes open-telemetry#11649 <!--Describe what testing was performed and which tests were added.--> #### Testing local build of ocb with locally downloaded converter and provider (can add this type of test to github actions if others think it is necessary) <!--Describe the documentation added.--> #### Documentation .chloggen file <!--Please delete paragraphs that you did not use before submitting.--> --------- Co-authored-by: Bogdan Drutu <bogdandrutu@gmail.com>
…ers (open-telemetry#11678) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description change name in templates and cmd/builder to refer to Providers and Converters as ConfmapProviders and ConfmapConverters <!-- Issue number if applicable --> #### Link to tracking issue as requested here: open-telemetry#11653 (comment) <!--Describe what testing was performed and which tests were added.--> #### Testing existing tests should cover this <!--Describe the documentation added.--> #### Documentation none, internal change and no new code/api <!--Please delete paragraphs that you did not use before submitting.-->
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Adds ability to replace Providers and Converters with local paths when building custom collector in ocb <!-- Issue number if applicable --> #### Link to tracking issue Closes open-telemetry#11649 <!--Describe what testing was performed and which tests were added.--> #### Testing local build of ocb with locally downloaded converter and provider (can add this type of test to github actions if others think it is necessary) <!--Describe the documentation added.--> #### Documentation .chloggen file <!--Please delete paragraphs that you did not use before submitting.--> --------- Co-authored-by: Bogdan Drutu <bogdandrutu@gmail.com>
…ers (open-telemetry#11678) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description change name in templates and cmd/builder to refer to Providers and Converters as ConfmapProviders and ConfmapConverters <!-- Issue number if applicable --> #### Link to tracking issue as requested here: open-telemetry#11653 (comment) <!--Describe what testing was performed and which tests were added.--> #### Testing existing tests should cover this <!--Describe the documentation added.--> #### Documentation none, internal change and no new code/api <!--Please delete paragraphs that you did not use before submitting.-->
Description
Adds ability to replace Providers and Converters with local paths when building custom collector in ocb
Link to tracking issue
Closes #11649
Testing
local build of ocb with locally downloaded converter and provider (can add this type of test to github actions if others think it is necessary)
Documentation
.chloggen file