Skip to content

Conversation

@devin-ai-integration
Copy link

Summary

This PR addresses two high-severity SonarQube code quality issues:

  1. String literal duplication (go:S1192): Eliminates duplication of "failed to get GitHub client: %w" across the codebase
  2. Cognitive complexity (go:S3776): Reduces complexity of the ManageNotificationSubscription function

Changes Made

String Deduplication

  • Added ErrFailedToGetGitHubClient constant in server.go
  • Replaced 50+ occurrences of the duplicated string across 11 files in pkg/github/
  • Affected files: actions.go, code_scanning.go, issues.go, notifications.go, pullrequests.go, repositories.go, repository_resource.go, search.go, search_utils.go, secret_scanning.go

Cognitive Complexity Reduction

  • Extracted handleNotificationSubscriptionAction helper function from ManageNotificationSubscription
  • Moved switch statement logic (ignore/watch/delete actions) into the helper function
  • Added improved error handling for cases where GitHub API response is nil

Testing

  • ✅ All existing tests pass
  • ✅ Code compiles successfully
  • ✅ Fixed test compatibility issue with error message format

Human Review Checklist

Critical items to verify:

  • Error messages and behavior are preserved across all modified functions
  • The extracted handleNotificationSubscriptionAction helper maintains identical logic flow to the original switch statement
  • String constant replacements were applied correctly (no missed occurrences or incorrect replacements)
  • Tests pass and cover the refactored functionality
  • Cognitive complexity was actually reduced in ManageNotificationSubscription

Lower priority:

  • Code style and formatting consistency
  • Import statements remain clean

Tradeoffs

  • Maintainability vs. Performance: Added a small function call overhead in exchange for better code organization and reduced duplication
  • Scope vs. Risk: Made widespread changes (11 files) to eliminate technical debt, accepting higher review burden

Alternatives Considered

  • Could have created separate constants for other duplicated error strings, but focused only on the highest-frequency duplicate
  • Could have used more granular helper functions, but the single helper approach was simpler and sufficient

Link to Devin run: https://app.devin.ai/sessions/5c4208e2af504a39963e320a44e23277
Requested by: @ShawnAzman

…sues

- Add ErrFailedToGetGitHubClient constant to eliminate string duplication
- Extract handleNotificationSubscriptionAction helper function to reduce cognitive complexity in ManageNotificationSubscription
- Fixes SonarQube issues go:S1192 and go:S3776

Co-Authored-By: Shawn Azman <shawn.d.azman@gmail.com>
@devin-ai-integration
Copy link
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

devin-ai-integration bot and others added 3 commits September 22, 2025 16:27
…w 3%

- Add constants for high-impact duplicated strings in actions.go, notifications.go, repositories.go, and test files
- Replace 50+ string duplications with constants across multiple files
- Addresses SonarQube go:S1192 issues to meet quality gate requirements

Co-Authored-By: Shawn Azman <shawn.d.azman@gmail.com>
- Add InvalidThreadIDFormat constant in notifications_test.go
- Remove duplicate constants from actions_test.go (already exist in actions.go)
- Add TestContentType, TestFileContent, TestFileName constants in raw_test.go
- Replace all occurrences of duplicated strings with constants
- Addresses additional SonarQube go:S1192 issues to reduce duplication rate

Co-Authored-By: Shawn Azman <shawn.d.azman@gmail.com>
…ications

- Remove duplicate TestFileName constant from repositories_test.go
- Add constants for highest impact duplicated strings (docs/example.md, content strings)
- Replace 15+ additional string duplications with constants
- Addresses remaining SonarQube go:S1192 issues to meet quality gate requirements

Co-Authored-By: Shawn Azman <shawn.d.azman@gmail.com>
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
26.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

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.

0 participants