Skip to content

Fix SonarQube S1192 and S3776 issues in pkg/github/notifications.go - #150

Open
eashansinha wants to merge 1 commit into
mainfrom
devin/1781126404-sonarqube-notifications
Open

eashansinha wants to merge 1 commit into
mainfrom
devin/1781126404-sonarqube-notifications

Conversation

@eashansinha

@eashansinha eashansinha commented Jun 10, 2026

Copy link
Copy Markdown

Closes: N/A (SonarQube remediation)

Summary

Fixes high-severity SonarQube issues in pkg/github/notifications.go:

S1192 — duplicated string literals replaced with package-level constants:

  • paramOwner, paramRepo, paramAction, paramNotificationID for repeated parameter names
  • errFailedToReadBodyFmt for the 4x-repeated "failed to read response body: %w" format string
  • errInvalidActionMessage, descRepositoryOwner, descRepositoryName

S3776 — cognitive complexity of the ListNotifications handler reduced by extracting:

  • parseListNotificationsParams(request) — extracts/validates the six optional params into a listNotificationsParams struct
  • buildNotificationListOptions(params) — builds github.NotificationListOptions including RFC3339 parsing of since/before

No behavior changes; error messages and tool schemas are identical.

Verification

  • go build ./..., go vet ./..., gofmt clean
  • go test ./pkg/github/ passes

Link to Devin session: https://app.devin.ai/sessions/1db4c7c8dff14301acfe40dde84da912
Requested by: @eashansinha


Devin Review

Status Commit
🟢 Reviewed fe58c75
Open in Devin Review (Staging)

Co-Authored-By: Eashan Sinha <eashan.sinha@cognition.ai>
@devin-ai-integration

Copy link
Copy Markdown
Original prompt from Eashan

Fix the following high severity SonarQube issues in @COG-GTM/github-mcp-server :
Repository: COG-GTM/github-mcp-server File: pkg/github/notifications.go Issues (8 total):

Fix these issues by:
For S1192 (duplicated strings): Define constants at the package level for repeated string literals
For S3776 (cognitive complexity): Refactor complex methods by extracting helper functions
Run tests after fixes to ensure nothing breaks.

@devin-ai-integration

Copy link
Copy Markdown

🤖 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, CI, and merge conflict monitoring

@sonarqubecloud

Copy link
Copy Markdown

@staging-devin-ai-integration staging-devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review (Staging)
Debug

Playground

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

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.

1 participant