Skip to content

Fix SonarQube string literal duplication issues#1499

Open
PabloHiro wants to merge 1 commit intoansible:develfrom
PabloHiro:fix-string-literal-duplications
Open

Fix SonarQube string literal duplication issues#1499
PabloHiro wants to merge 1 commit intoansible:develfrom
PabloHiro:fix-string-literal-duplications

Conversation

@PabloHiro
Copy link
Copy Markdown
Contributor

@PabloHiro PabloHiro commented Dec 5, 2025

AAP-60060

Summary

This PR addresses SonarQube string literal duplication issues by extracting repeated string literals into named constants.

Changes

  • Extracted repeated string literals into constants across multiple files
  • Improved code maintainability by reducing duplication

Files Modified

  • pkg/certificates/cli.go
  • pkg/netceptor/external_backend.go
  • pkg/netceptor/netceptor.go
  • pkg/workceptor/command.go
  • pkg/workceptor/controlsvc.go
  • pkg/workceptor/kubernetes.go
  • pkg/workceptor/remote_work.go

Test Plan

  • Existing tests should continue to pass
  • No functional changes, only code quality improvements

@PabloHiro PabloHiro force-pushed the fix-string-literal-duplications branch from caf6208 to a16f156 Compare December 5, 2025 12:40
Extract repeated string literals into named constants to improve
code maintainability and reduce duplication across multiple files:
- pkg/certificates/cli.go
- pkg/netceptor/external_backend.go
- pkg/netceptor/netceptor.go
- pkg/workceptor/command.go
- pkg/workceptor/controlsvc.go
- pkg/workceptor/kubernetes.go
- pkg/workceptor/remote_work.go

Assisted-by: Claude <noreply@anthropic.com>
@PabloHiro PabloHiro force-pushed the fix-string-literal-duplications branch from a16f156 to aba4339 Compare December 5, 2025 12:47
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Dec 5, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
34.5% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Copy link
Copy Markdown
Contributor

@arrestle arrestle left a comment

Choose a reason for hiding this comment

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

Hi Pablo,
I noticed we can improve our error handling in this PR, I'm ok with flagging it in a new Jira if it these changes cause code coverage issues, however., so I'm marking it as Approved in the hopes you'll either enhance the message, or log a jira to handle it later, since it's a fairly minor change.

Again, thanks for being our SonarQube liason!

func (s *Netceptor) AddWorkCommand(command string, secure bool) error {
if command == "" {
return fmt.Errorf("must provide a name")
return errors.New(errMustProvideName)
Copy link
Copy Markdown
Contributor

@arrestle arrestle Dec 5, 2025

Choose a reason for hiding this comment

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

Hmmm. I dislike that all these error messages are identical. Can we take this opportunity to mention which name is missing?

Suggested change
return errors.New(errMustProvideName)
return errors.fmt(errMustProvideName,"Work Command")

func (s *Netceptor) SetServerTLSConfig(name string, config *tls.Config) error {
if name == "" {
return fmt.Errorf("must provide a name")
return errors.New(errMustProvideName)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
return errors.New(errMustProvideName)
return errors.fmt(errMustProvideName,"Server TLS Config")

// defaultMaxConnectionIdleTime is the maximum time a connection can go without data before we consider it failed.
const defaultMaxConnectionIdleTime = 2*defaultRouteUpdateTime + 1*time.Second

const errMustProvideName = "must provide a name"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
const errMustProvideName = "must provide a name"
const errMustProvideName = "must provide a %s name"


const errMsgStatusFileUpdate = "Error updating status file %s: %s"
const (
errMsgStatusFileUpdate = "Error updating status file %s: %s"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Because I see this error message has parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants