-
Notifications
You must be signed in to change notification settings - Fork 109
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
Move condition logic to main module #1618
Conversation
@@ -31,28 +33,19 @@ const ( | |||
DatadogAgentStateFailed DatadogAgentState = "Failed" | |||
) | |||
|
|||
// UpdateDatadogAgentStatusConditionsFailure used to update the failure StatusConditions |
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.
For reviewers: I deleted this function and others that were not used.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1618 +/- ##
==========================================
- Coverage 49.28% 48.82% -0.47%
==========================================
Files 216 217 +1
Lines 20662 20882 +220
==========================================
+ Hits 10184 10196 +12
- Misses 9942 10149 +207
- Partials 536 537 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
To avoid depending on EDS in the api module
5a609e1
to
24adfda
Compare
go.mod
Outdated
@@ -43,6 +43,7 @@ require ( | |||
github.com/DataDog/datadog-agent/pkg/config/model v0.59.0-rc.5 | |||
github.com/DataDog/datadog-agent/pkg/config/remote v0.59.0-rc.5 | |||
github.com/DataDog/datadog-agent/pkg/remoteconfig/state v0.59.0-rc.5 | |||
github.com/DataDog/datadog-operator/api v0.0.0-20250113195927-3d36de7daa54 |
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 this is correct. This dependency should by managed by go work
without pinning to a specific version.
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 added a new commit with the fix.
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.
Thanks for catching this!
What does this PR do?
Moves the logic in
api/datadoghq/v2alpha1/condition.go
to the main module.That file depends on EDS and we'd rather avoid having that dependency in the api module. Otherwise, projects importing it will indirectly depend on EDS too. It makes sense to move this logic to the main module because it doesn't have any CRD definitions, which is the point of the api module.
Describe your test plan
It's a refactor. Tests should be enough.
Checklist
bug
,enhancement
,refactoring
,documentation
,tooling
, and/ordependencies
qa/skip-qa
label