-
Notifications
You must be signed in to change notification settings - Fork 2.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
[extension/ackextension] release ackextension in alpha stability #32308
[extension/ackextension] release ackextension in alpha stability #32308
Conversation
462212e
to
3c24fd7
Compare
d1b0d7c
to
f932bcb
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.
v0.98.0
was just released, so version numbers here should be updated to match 👍
d88f164
to
3160c6e
Compare
My apologies, you're right, it's not clear. It's being generated from here, so you'd actually want to update the builder config. I believe you'll also want to add the extension to the extension tests here as well. |
398f935
to
a86f99c
Compare
33c79a4
to
285b70b
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.
I believe you also want to add the component to the extensions block near the top of this file.
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.
Ok I added the component. make checks
seemed to have resolved my component to version v0.0.0-00010101000000-000000000000. I assume this is normal given that the extension hasn't been released yet. Is this the right version to use?
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 don't think it matters either way as the replace will simply override whatever is stated with the current contents of the relative dir below. However, could you test to see if CI passes with v0.98.0
? (Same with other v0.0...
reference in cmd/otelcontribcol/go.mod
)
Hi @dmitryax, minding taking a look? |
beadcfd
to
a5a1fe0
Compare
f48483a
to
907f708
Compare
@@ -20,6 +20,8 @@ import ( | |||
"go.opentelemetry.io/collector/extension/extensiontest" | |||
"go.opentelemetry.io/collector/extension/zpagesextension" | |||
|
|||
"github.com/open-telemetry/opentelemetry-collector-contrib/extension/ackextension" | |||
|
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.
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 don't think it matters either way as the replace will simply override whatever is stated with the current contents of the relative dir below. However, could you test to see if CI passes with v0.98.0
? (Same with other v0.0...
reference in cmd/otelcontribcol/go.mod
)
65e421a
to
f8bd589
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.
LGTM
Description:
release ackextension in alpha stability so other components can start using it.
Link to tracking Issue:
#26376
Testing:
Documentation: