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

Refactor utility functions related to pointers #1710

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

shuheiktgw
Copy link
Collaborator

What does this PR do?

Most of the pointer-related utility functions have been deprecated due to the introduction of generics. In this PR, I've refactored the code and removed most of them.

Motivation

Cleaning up the codebase.

Additional Notes

N/A

Minimum Agent Versions

N/A

Describe your test plan

N/A

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

@shuheiktgw shuheiktgw requested review from a team as code owners February 19, 2025 07:24
@@ -85,7 +85,7 @@ func (f *admissionControllerFeature) ID() feature.IDType {
return feature.AdmissionControllerIDType
}
func shouldEnablesidecarInjection(sidecarInjectionConf *v2alpha1.AgentSidecarInjectionConfig) bool {
if sidecarInjectionConf != nil && sidecarInjectionConf.Enabled != nil && apiutils.BoolValue(sidecarInjectionConf.Enabled) {
if sidecarInjectionConf != nil && sidecarInjectionConf.Enabled != nil && apiutils.NewDeref(sidecarInjectionConf.Enabled, false) {

Choose a reason for hiding this comment

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

🔵 Code Quality Violation

Condition could be directly returned (...read more)

In Go, it is considered unnecessary and less readable to write a conditional statement that returns true or false explicitly based on a condition. Instead, it is recommended to directly return the condition itself. Here's why:

  1. Code simplicity and readability: Writing return condition directly conveys the intent of the code more clearly and reduces unnecessary verbosity. It is easier for other developers to understand your code at a glance without having to analyze additional conditional statements.
  2. Avoidance of redundancy: Explicitly returning true or false based on a condition introduces redundancy in the code. Since the condition itself already evaluates to a boolean value (true or false), there is no need to include additional return true or return false statements.
  3. Maintainability and refactoring: The direct return condition approach is more maintainable and flexible for future code changes. If the condition or the desired return value changes, it is easier to modify a single line rather than multiple lines of code. This minimizes the chances of introducing errors or inconsistencies during refactoring.

Therefore, it is recommended to simply use return condition instead of if condition { return true } return false. By doing so, you improve code readability, reduce redundancy, and ensure better maintainability of your Go code.

View in Datadog  Leave us feedback  Documentation

}

// SingleStepInstrumentation requires APM Enabled
if apmConf.SingleStepInstrumentation != nil && apiutils.BoolValue(apmConf.SingleStepInstrumentation.Enabled) {
if apmConf.SingleStepInstrumentation != nil && apiutils.NewDeref(apmConf.SingleStepInstrumentation.Enabled, false) {

Choose a reason for hiding this comment

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

🔵 Code Quality Violation

Condition could be directly returned (...read more)

In Go, it is considered unnecessary and less readable to write a conditional statement that returns true or false explicitly based on a condition. Instead, it is recommended to directly return the condition itself. Here's why:

  1. Code simplicity and readability: Writing return condition directly conveys the intent of the code more clearly and reduces unnecessary verbosity. It is easier for other developers to understand your code at a glance without having to analyze additional conditional statements.
  2. Avoidance of redundancy: Explicitly returning true or false based on a condition introduces redundancy in the code. Since the condition itself already evaluates to a boolean value (true or false), there is no need to include additional return true or return false statements.
  3. Maintainability and refactoring: The direct return condition approach is more maintainable and flexible for future code changes. If the condition or the desired return value changes, it is easier to modify a single line rather than multiple lines of code. This minimizes the chances of introducing errors or inconsistencies during refactoring.

Therefore, it is recommended to simply use return condition instead of if condition { return true } return false. By doing so, you improve code readability, reduce redundancy, and ensure better maintainability of your Go code.

View in Datadog  Leave us feedback  Documentation

@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2025

Codecov Report

Attention: Patch coverage is 69.55224% with 102 lines in your changes missing coverage. Please review.

Project coverage is 49.30%. Comparing base (bd4ad0b) to head (002b257).

Files with missing lines Patch % Lines
internal/controller/testutils/agent.go 0.00% 64 Missing ⚠️
pkg/testutils/builder.go 89.89% 10 Missing ⚠️
...adogagent/component/clusterchecksrunner/default.go 0.00% 4 Missing ⚠️
pkg/constants/utils.go 0.00% 4 Missing ⚠️
...troller/datadogagent/controller_reconcile_agent.go 0.00% 1 Missing and 1 partial ⚠️
...ontroller/datadogagent/controller_reconcile_ccr.go 0.00% 0 Missing and 2 partials ⚠️
...atadogagent/feature/kubernetesstatecore/feature.go 60.00% 1 Missing and 1 partial ⚠️
...nternal/controller/datadogagent/override/global.go 66.66% 1 Missing and 1 partial ⚠️
internal/controller/datadogagent/utils.go 0.00% 2 Missing ⚠️
...ontroller/datadogagent/controller_reconcile_dca.go 0.00% 0 Missing and 1 partial ⚠️
... and 9 more

❌ Your patch status has failed because the patch coverage (69.55%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1710      +/-   ##
==========================================
- Coverage   49.30%   49.30%   -0.01%     
==========================================
  Files         219      219              
  Lines       21298    21292       -6     
==========================================
- Hits        10501    10498       -3     
+ Misses      10250    10248       -2     
+ Partials      547      546       -1     
Flag Coverage Δ
unittests 49.30% <69.55%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
...controller/datadogagent/component/agent/default.go 79.35% <100.00%> (ø)
...ler/datadogagent/component/clusteragent/default.go 96.89% <100.00%> (ø)
...nal/controller/datadogagent/feature/asm/feature.go 71.21% <100.00%> (ø)
...roller/datadogagent/feature/autoscaling/feature.go 72.91% <100.00%> (ø)
...ller/datadogagent/feature/clusterchecks/feature.go 62.04% <100.00%> (ø)
...al/controller/datadogagent/feature/cspm/feature.go 75.20% <100.00%> (ø)
...ntroller/datadogagent/feature/ebpfcheck/feature.go 83.60% <100.00%> (ø)
...er/datadogagent/feature/eventcollection/feature.go 62.31% <100.00%> (ø)
...er/datadogagent/feature/externalmetrics/feature.go 54.16% <100.00%> (ø)
...nal/controller/datadogagent/feature/gpu/feature.go 90.47% <100.00%> (ø)
... and 36 more

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 bd4ad0b...002b257. Read the comment docs.

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