Skip to content
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

[CECO-1860][DatadogGenericResource] Refactor handler 2 #1640

Closed
wants to merge 14 commits into from

Conversation

tbavelier
Copy link
Member

What does this PR do?

Refactors CRUD operations

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Minimum Agent Versions

Are there minimum versions of the Datadog Agent and/or Cluster Agent required?

  • Agent: vX.Y.Z
  • Cluster Agent: vX.Y.Z

Describe your test plan

QA to be done on the "main" PR that will be merged

Checklist

  • PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
  • PR has a milestone or the qa/skip-qa label

@codecov-commenter
Copy link

codecov-commenter commented Jan 24, 2025

Codecov Report

Attention: Patch coverage is 4.76190% with 120 lines in your changes missing coverage. Please review.

Project coverage is 49.45%. Comparing base (b8dad05) to head (c67ea9e).

Files with missing lines Patch % Lines
...nternal/controller/datadoggenericresource/utils.go 4.00% 48 Missing ⚠️
...al/controller/datadoggenericresource/synthetics.go 0.00% 47 Missing ⚠️
...nal/controller/datadoggenericresource/notebooks.go 13.79% 25 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1640      +/-   ##
==========================================
- Coverage   49.49%   49.45%   -0.05%     
==========================================
  Files         218      218              
  Lines       21244    21267      +23     
==========================================
+ Hits        10515    10517       +2     
- Misses      10182    10203      +21     
  Partials      547      547              
Flag Coverage Δ
unittests 49.45% <4.76%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...nal/controller/datadoggenericresource/notebooks.go 11.47% <13.79%> (+0.36%) ⬆️
...al/controller/datadoggenericresource/synthetics.go 37.75% <0.00%> (+6.66%) ⬆️
...nternal/controller/datadoggenericresource/utils.go 53.39% <4.00%> (-39.59%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b8dad05...c67ea9e. Read the comment docs.

@tbavelier tbavelier added this to the v1.13.0 milestone Feb 10, 2025
Base automatically changed from tbavelier/levan-m/generic-resoource-refactor to main February 11, 2025 16:17
"errors"
"fmt"
"net/url"
"strconv"
"strconv"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Quality Violation

Verify that importing the same package multiple times is necessary. (...read more)

In Go, duplicate imports refer to importing the same package multiple times in a single file. It is considered a best practice to avoid duplicate imports in Go for the following reasons:

  1. Code readability and maintainability: Duplicate imports can make code harder to read and understand. When the same package is imported multiple times, it can lead to confusion and make it more difficult to determine the source of a particular symbol or function. Keeping imports concise and free of duplicates helps improve code readability and maintainability.
  2. Name conflicts: Duplicate imports introduce the risk of name conflicts. If the same package is imported multiple times, Go does not distinguish between them, which can result in name clashes between symbols from different imports. This can cause compilation errors or unexpected behavior, making the code prone to bugs and difficult to troubleshoot.
  3. Package initialization: Each package in Go can have an initialization function, init(), which is executed during package initialization. When the same package is imported multiple times, the init() function is run multiple times as well. This can lead to unexpected side effects and violate assumptions made by the package initialization code.
  4. Compilation efficiency: Duplicate imports can impact compilation time and increase the size of the resulting binary. The Go compiler needs to process each imported package, and duplicating imports can cause unnecessary overhead during the build process.

To avoid these issues, it is recommended to keep imports concise and remove any duplicates. Go provides a handy feature where you can group multiple imports from the same package on a single line, reducing duplication. Additionally, using aliases when necessary can help resolve naming conflicts between symbols from different packages.

View in Datadog  Leave us feedback  Documentation

@tbavelier tbavelier force-pushed the tbavelier/generic_resource_re_refactor branch from 0fdb552 to 090df80 Compare February 11, 2025 16:49
@tbavelier tbavelier closed this Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants