-
Notifications
You must be signed in to change notification settings - Fork 208
Fix missing artifact warning #3900
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
Conversation
agent/instrumentation/applicationinsights-web-2.3/build.gradle.kts
Outdated
Show resolved
Hide resolved
| compileOnly("com.microsoft.azure:applicationinsights-web:2.3.0") | ||
|
|
||
| testImplementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-annotations:$otelVersion") | ||
| testImplementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-annotations:$otelInstrumentationVersion") |
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 put a note saying when it's moved from sdk to otelInstrumentation?
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 put a note saying when it's moved from sdk to otelInstrumentation?
I didn't follow this comment, can you clarify? thanks
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.
this has been there for 2 years. I just want to know when it was moved to a different artifact and put a comment there.
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.
ah, it has always been wrong
it just happened to work because before we bumped instrumentation repo to 2.x it generally had the same version number as the SDK repo
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.
plus, it's still "working" (CI passes), it just logs a warning which I think S360 may be complaining about
| } | ||
|
|
||
| val otelInstrumentationAlphaVersion: String by project | ||
| val otelVersion: String by project |
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 rename otelVersion from dependencyManagement to otelSdkVersion?
otelVersion seems confusing.
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.
sure, can I send a follow-up PR for that since it's unrelated to this PR?
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.
np
I think this warning that mentions
repo.maven.apache.orgmay be what's triggering (one of) the current S360 violations: