- Notifications
You must be signed in to change notification settings - Fork1.1k
Feature/chewy standards tool#510
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
Adds a new MCP tool for fetching development standards from Chewy’s GitHub repository, including registration, implementation, tests, and documentation updates.
- Introduces
GetChewyStandardstool and registers it inInitToolsets - Implements handler logic to fetch either an explicit path or the full repository tree
- Adds comprehensive tests and updates the README
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/github/tools.go | Registersget_chewy_standards in the read-only toolset |
| pkg/github/repositories.go | ImplementsGetChewyStandards handler and tool definition |
| pkg/github/repositories_test.go | Adds tests for default, custom-path, and error cases |
| README.md | Documents the newget_chewy_standards tool |
Comments suppressed due to low confidence (3)
pkg/github/repositories_test.go:1968
- Add a test case that supplies a custom
refvalue and assert that the handler uses that ref when fetching contents.
func Test_GetChewyStandards(t *testing.T) {README.md:512
- Align the bullet levels and indentation for the
get_chewy_standardssection so it matches the existing format, and ensure each sub-item is nested correctly.
- **get_chewy_standards** - Get development standards and guidelines from Chewy's repositoryREADME.md:604
- Remove the stray trailing `- `` at the end of the file; it appears to be an accidental leftover.
- `| if searchWholeRepo { | ||
| // Get the entire repository tree | ||
| tree, resp, err := client.Git.GetTree(ctx, owner, repo, "HEAD", true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Use the user-providedref parameter when fetching the repository tree instead of hardcoding "HEAD" so that branch/tag/commit references are respected.
| tree,resp,err:=client.Git.GetTree(ctx,owner,repo,"HEAD",true) | |
| tree,resp,err:=client.Git.GetTree(ctx,owner,repo,ref,true) |
Copilot uses AI. Check for mistakes.
| defer func() { _ = resp.Body.Close() }() | ||
| } | ||
| if resp.StatusCode == 200 && tree != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Add a check for non-200 responses fromGetTree (e.g.resp.StatusCode != http.StatusOK) and return an error, rather than continuing silently when the tree isn’t fetched.
| ifresp.StatusCode==200&&tree!=nil { | |
| ifresp.StatusCode!=http.StatusOK { | |
| returnmcp.NewToolResultError(fmt.Sprintf("Failed to get repository tree: received status code %d",resp.StatusCode)),nil | |
| } | |
| iftree!=nil { |
Copilot uses AI. Check for mistakes.
| continue | ||
| } | ||
| if resp != nil { | ||
| defer func() { _ = resp.Body.Close() }() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Avoid deferringresp.Body.Close() inside a loop; close the response body immediately after reading to prevent resource buildup.
Copilot uses AI. Check for mistakes.
Closes: