chore: update CODEOWNERS with apm-sdk-api-go team#3950
Conversation
|
/merge |
|
View all feedbacks in Devflow UI.
This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
[email protected] unqueued this merge request |
There was a problem hiding this comment.
Would love to discuss a bit about codeowner in the next guild meeting
SDK Capabilities should be responsible (or knowledgeable) for the following [...]
Reading this makes me feel like its putting the cart before the horses. Has a case happened where changes in APM SDK code where merged because someone bypassed you?
I would also love to know where we would need to ask for review in case this team is in the codeowners
| /ddtrace/tracer/remote_config.go @DataDog/apm-sdk-api-go | ||
| /ddtrace/tracer/otel_dd_mappings.go @DataDog/apm-sdk-api-go | ||
| # capabilities - telemetry | ||
| /internal/telemetry @DataDog/apm-sdk-api-go |
There was a problem hiding this comment.
I would love to still have the ability to do changes in my own code 😅
There was a problem hiding this comment.
💯 even if our team is supposed to be owning this it was 100% written by you. I will comment it out explaining why.
| # capabilities - config, dynamic config, remote config | ||
| /internal/env @DataDog/apm-sdk-api-go | ||
| /internal/stableconfig @DataDog/apm-sdk-api-go | ||
| /internal/globalconfig @DataDog/apm-sdk-api-go |
There was a problem hiding this comment.
Is this very useful to guard packages like this with one very simple file?
There was a problem hiding this comment.
We could argue for both yes and no as it's indeed a simple package.
I mostly took everything related to the scope of the sdk capabilities team and added it as codeowners.
There was a problem hiding this comment.
Yes I understand. My idea here is just to understand what triggered this the need for this and how it will impact adjacent team like us. If you are okay talking about it during the next guild meeting it would be awesome
|
/merge -c |
|
View all feedbacks in Devflow UI.
|
dd-devflow
Bot
added
mergequeue-status: removed
and removed
mergequeue-status: waiting
labels
|
More context about the "why" of this PR. From what I understood when I got the request APM has a monthly report where multiple OKRs are tracked. Some of those OKR are related to CI failures and correlated with codeowners. This is currently done for python and java. There has never been an occurence where something was merged without our greenlight or not us being aware. I still think that this is an improvement as it could clarify which team "should be" responsible for a given part of the tracer even if we are all working on it. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Tests failed on this commit 04009de:
What to do next?
|
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
dd-devflow
Bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: rejected
mergequeue-status: queued
labels
dd-devflow
Bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
3 participants
What does this PR do?
Add the SDK Capabilities team to CODEOWNERS for the area the team is responsible. I used the APM DCS organization page on confluence as reference.
SDK Capabilities should be responsible (or knowledgeable) for the following: config, instrumentation telemetry, runtime metrics, sampling, baggage, trace context, drop-in support, OTel Traces/Metrics/Logs
Motivation
Reviewer's Checklist
./scripts/lint.shlocally.Unsure? Have a question? Request a review!