Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/lint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
go mod verify
go mod download
LINT_VERSION=1.64.8
LINT_VERSION=2.1.6
curl -fsSL https://github.com/golangci/golangci-lint/releases/download/v${LINT_VERSION}/golangci-lint-${LINT_VERSION}-linux-amd64.tar.gz | \
tar xz --strip-components 1 --wildcards \*/golangci-lint
mkdir -p bin && mv golangci-lint bin/
Expand All @@ -45,6 +45,6 @@ jobs:
assert-nothing-changed go fmt ./...
assert-nothing-changed go mod tidy
bin/golangci-lint run --out-format=colored-line-number --timeout=3m || STATUS=$?
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the replacement, if any, for --out-format=colored-line-number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

colored-line-number has been enabled by default.

I suppose the --out-format has been removed.

bin/golangci-lint run --timeout=3m || STATUS=$?
exit $STATUS
33 changes: 26 additions & 7 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# https://golangci-lint.run/usage/configuration
version: "2"

run:
timeout: 5m
tests: true
Expand All @@ -8,21 +11,37 @@ linters:
- govet
- errcheck
- staticcheck
- gofmt
Copy link
Collaborator

Choose a reason for hiding this comment

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

@radar07 can you talk me through these removals? Are they somewhere else? Were they removed? Are they in core?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

- goimports
- revive
- ineffassign
- typecheck
- unused
- gosimple
- misspell
- nakedret
- bodyclose
- gocritic
- makezero
- gosec
settings:
staticcheck:
checks:
- all
- '-QF1008' # Allow embedded structs to be referenced by field
- '-ST1000' # Do not require package comments
revive:
rules:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Turned off a few code comment rules. Not that they are wrong, but because we should turn them on intentionally rather than just because we're doing an upgrade.

- name: exported
disabled: true
- name: exported
disabled: true
- name: package-comments
disabled: true

formatters:
enable:
- gofmt
Copy link
Collaborator

Choose a reason for hiding this comment

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

@radar07 I removed gci and golines. I like them but they aren't included in most people's workflows and the errors can be opaque for example:

internal/ghmcp/server.go:17:1: File is not properly formatted (gci)

^

Totally unclear how to fix unless you understand what gci is. Not worth the hassle.

- goimports

output:
formats: colored-line-number
print-issued-lines: true
print-linter-name: true
formats:
text:
print-linter-name: true
print-issued-lines: true
13 changes: 7 additions & 6 deletions cmd/mcpcurl/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ type (
Arguments map[string]interface{} `json:"arguments"`
}

// Define structure to match the response format
// Content matches the response format of a text content response
Content struct {
Type string `json:"type"`
Text string `json:"text"`
Expand Down Expand Up @@ -284,10 +284,10 @@ func addCommandFromTool(toolsCmd *cobra.Command, tool *Tool, prettyPrint bool) {
cmd.Flags().Bool(name, false, description)
case "array":
if prop.Items != nil {
if prop.Items.Type == "string" {
switch prop.Items.Type {
case "string":
cmd.Flags().StringSlice(name, []string{}, description)
} else if prop.Items.Type == "object" {
// For complex objects in arrays, we'll use a JSON string that users can provide
case "object":
cmd.Flags().String(name+"-json", "", description+" (provide as JSON array)")
}
}
Expand Down Expand Up @@ -327,11 +327,12 @@ func buildArgumentsMap(cmd *cobra.Command, tool *Tool) (map[string]interface{},
}
case "array":
if prop.Items != nil {
if prop.Items.Type == "string" {
switch prop.Items.Type {
case "string":
if values, _ := cmd.Flags().GetStringSlice(name); len(values) > 0 {
arguments[name] = values
}
} else if prop.Items.Type == "object" {
case "object":
if jsonStr, _ := cmd.Flags().GetString(name + "-json"); jsonStr != "" {
var jsonArray []interface{}
if err := json.Unmarshal([]byte(jsonStr), &jsonArray); err != nil {
Expand Down
3 changes: 1 addition & 2 deletions internal/ghmcp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/github/github-mcp-server/pkg/translations"
gogithub "github.com/google/go-github/v69/github"
"github.com/mark3labs/mcp-go/mcp"

"github.com/mark3labs/mcp-go/server"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -170,7 +169,7 @@ func RunStdioServer(cfg StdioServerConfig) error {

logrusLogger := logrus.New()
if cfg.LogFilePath != "" {
file, err := os.OpenFile(cfg.LogFilePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
file, err := os.OpenFile(cfg.LogFilePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0600)
if err != nil {
return fmt.Errorf("failed to open log file: %w", err)
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/github/repositories.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ func CreateBranch(getClient GetClientFn, t translations.TranslationHelperFunc) (
if err != nil {
return nil, fmt.Errorf("failed to get repository: %w", err)
}
defer resp.Body.Close()
defer func() { _ = resp.Body.Close() }()

fromBranch = *repository.DefaultBranch
}
Expand All @@ -622,7 +622,7 @@ func CreateBranch(getClient GetClientFn, t translations.TranslationHelperFunc) (
if err != nil {
return nil, fmt.Errorf("failed to get reference: %w", err)
}
defer resp.Body.Close()
defer func() { _ = resp.Body.Close() }()

// Create new branch
newRef := &github.Reference{
Expand All @@ -634,7 +634,7 @@ func CreateBranch(getClient GetClientFn, t translations.TranslationHelperFunc) (
if err != nil {
return nil, fmt.Errorf("failed to create branch: %w", err)
}
defer resp.Body.Close()
defer func() { _ = resp.Body.Close() }()

r, err := json.Marshal(createdRef)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/translations/translations.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TranslationHelper() (TranslationHelperFunc, func()) {
}
}

// dump translationKeyMap to a json file called github-mcp-server-config.json
// DumpTranslationKeyMap writes the translation map to a json file called github-mcp-server-config.json
func DumpTranslationKeyMap(translationKeyMap map[string]string) error {
file, err := os.Create("github-mcp-server-config.json")
if err != nil {
Expand Down