forked from MetLife/github-mcp-server
-
Couldn't load subscription status.
- Fork 0
Resolve code duplication by extracting string literals to constants #24
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
Open
devin-ai-integration
wants to merge
7
commits into
main
Choose a base branch
from
devin/1758561595-resolve-code-duplication
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Resolve code duplication by extracting string literals to constants #24
devin-ai-integration
wants to merge
7
commits into
main
from
devin/1758561595-resolve-code-duplication
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Created pkg/github/constants.go with common error messages and parameter descriptions - Replaced 'failed to get GitHub client: %w' with ErrFailedToGetGitHubClient constant - Replaced 'failed to marshal response: %w' with ErrFailedToMarshalResponse constant - Replaced 'failed to read response body: %w' with ErrFailedToReadResponseBody constant - Replaced 'Repository owner' with ParamRepositoryOwner constant - Replaced 'Repository name' with ParamRepositoryName constant - Replaced 'Pull request number' with ParamPullRequestNumber constant - Replaced 'The unique identifier of the workflow run' with ParamWorkflowRunID constant - Removed duplicate constants from actions.go and consolidated with main constants.go This addresses the 29% code duplication identified in SonarQube analysis and improves code maintainability. Co-Authored-By: Eashan Sinha <eashan.sinha@codeium.com>
- Replace remaining instances of 'The number of results per page (max 100)' with ParamResultsPerPage constant - Replace 'The page number of the results to fetch' with ParamPageNumber constant This completes the code duplication resolution identified in SonarQube analysis. Co-Authored-By: Eashan Sinha <eashan.sinha@codeium.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
- Replace last hardcoded 'Repository name' string with ParamRepositoryName constant - This completes the code duplication resolution task Co-Authored-By: Eashan Sinha <eashan.sinha@codeium.com>
- Replace remaining instances of 'Sort order' with ParamSortOrder constant in issues.go and search.go - This addresses additional string literal duplication identified in SonarQube analysis Co-Authored-By: Eashan Sinha <eashan.sinha@codeium.com>
- Create WithPaginationPerPage() helper for functions expecting per_page parameters - Replace remaining custom parameter definitions with helper functions - Update toolsnaps to match new pagination schemas - Ensure actions.go and GetIssueComments use per_page parameters - Ensure all other functions use standard perPage parameters with constraints - All tests now pass with consistent pagination schemas Co-Authored-By: Eashan Sinha <eashan.sinha@codeium.com>
- Replace remaining hardcoded parameter descriptions with constants - Ensure consistent use of ParamRepositoryOwner, ParamRepositoryName, etc. - Address duplication in code_scanning.go, notifications.go, pullrequests.go, repositories.go, secret_scanning.go Co-Authored-By: Eashan Sinha <eashan.sinha@codeium.com>
…duplication - Replace repetitive MCP tool creation patterns with CreateBasicTool, CreateWorkflowTool, CreateWorkflowIDTool helpers - Standardize error handling with HandleStandardToolError and ValidateWorkflowParams/ValidateWorkflowIDParams - Remove duplicate ToBoolPtr function from tools.go (already exists in constants.go) - Significantly reduce structural duplication in parameter definitions and validation patterns - Target SonarCloud duplication reduction from 53.9% to ≤3% on new code Co-Authored-By: Eashan Sinha <eashan.sinha@codeium.com>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.


Summary
Resolves code duplication by extracting repeated string literals into constants, addressing the 29% code duplication identified in SonarQube analysis.
Changes
pkg/github/constants.gowith centralized error messages and parameter descriptions"failed to get GitHub client: %w"→ErrFailedToGetGitHubClient"failed to marshal response: %w"→ErrFailedToMarshalResponse"failed to read response body: %w"→ErrFailedToReadResponseBody"Repository owner"→ParamRepositoryOwner"Repository name"→ParamRepositoryName"Pull request number"→ParamPullRequestNumber"The unique identifier of the workflow run"→ParamWorkflowRunID"The number of results per page (max 100)"→ParamResultsPerPage"The page number of the results to fetch"→ParamPageNumberactions.goand consolidated with main constants fileTesting
go test ./...)go build ./...)Review Focus
Please pay special attention to:
Tradeoffs
Link to Devin run: https://app.devin.ai/sessions/f89f0b3ffdfe44b28208818907af7126
Requested by: @eashansinha
This addresses the SonarQube code quality analysis findings and should significantly reduce the reported 29% code duplication in the repository.