From d9e9ab41819fa68bef547d5d75d6b07117180f90 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Wed, 18 Mar 2026 09:23:21 +0000 Subject: [PATCH] Fix lint issues from golangci-lint quality checks Replace magic numbers with named constants, extract repeated string literals into constants, convert if-else chains to switch statements, replace unnecessary lambdas with direct function references, simplify HasPrefix+TrimPrefix to CutPrefix, mark unused parameters, and reduce cognitive complexity by extracting helper functions. --- bitbucket/bitbucket.go | 2 +- bitbucket/issues.go | 17 +++--- bitbucket/prs.go | 4 +- forge.go | 8 +-- gitea/commit_statuses.go | 6 ++- gitea/gitea.go | 8 +-- gitea/issues.go | 14 ++--- gitea/labels.go | 10 ++-- gitea/milestones.go | 16 ++++-- gitea/notifications.go | 96 ++++++++++++++++++++-------------- gitea/prs.go | 21 ++++---- gitea/rate_limit.go | 2 +- gitea/releases.go | 4 +- github/ci.go | 4 +- github/collaborators.go | 26 +++++---- github/commit_statuses.go | 2 +- github/github.go | 10 ++-- github/issues.go | 11 ++-- github/milestones.go | 4 +- github/notifications.go | 84 +++++++++++++++++------------ github/prs.go | 8 +-- github/reactions.go | 4 +- github/secrets.go | 6 ++- gitlab/commit_statuses.go | 4 +- gitlab/gitlab.go | 8 +-- gitlab/issues.go | 8 ++- gitlab/milestones.go | 8 ++- gitlab/notifications.go | 2 +- gitlab/prs.go | 20 ++++--- internal/cli/api.go | 8 +-- internal/cli/branch.go | 6 ++- internal/cli/ci.go | 6 ++- internal/cli/issue.go | 12 +++-- internal/cli/label.go | 4 +- internal/cli/milestone.go | 4 +- internal/cli/notification.go | 12 +++-- internal/cli/pr.go | 91 +++++++++++++++++--------------- internal/cli/release.go | 11 ++-- internal/cli/release_test.go | 8 +-- internal/cli/repo.go | 33 +++++++----- internal/cli/review.go | 14 +++-- internal/cli/search.go | 6 ++- internal/config/config.go | 9 +++- internal/config/config_test.go | 14 ++--- internal/output/output.go | 4 +- internal/resolve/resolve.go | 21 +++----- 46 files changed, 413 insertions(+), 267 deletions(-) diff --git a/bitbucket/bitbucket.go b/bitbucket/bitbucket.go index 8b9167e..031c64a 100644 --- a/bitbucket/bitbucket.go +++ b/bitbucket/bitbucket.go @@ -128,7 +128,7 @@ func (s *bitbucketRepoService) doJSON(ctx context.Context, method, url string, b if resp.StatusCode == http.StatusNoContent { return nil } - if resp.StatusCode >= 400 { + if resp.StatusCode >= http.StatusBadRequest { respBody, _ := io.ReadAll(resp.Body) return &forge.HTTPError{StatusCode: resp.StatusCode, URL: url, Body: string(respBody)} } diff --git a/bitbucket/issues.go b/bitbucket/issues.go index b2d94a9..f7e171e 100644 --- a/bitbucket/issues.go +++ b/bitbucket/issues.go @@ -8,6 +8,11 @@ import ( "time" ) +const ( + stateOpen = "open" + stateClosed = "closed" +) + type bitbucketIssueService struct { token string httpClient *http.Client @@ -104,10 +109,10 @@ func convertBitbucketIssue(bb bbIssue) forge.Issue { // Normalize Bitbucket states to open/closed switch bb.State { - case "new", "open": - result.State = "open" + case "new", stateOpen: + result.State = stateOpen default: - result.State = "closed" + result.State = stateClosed } if bb.Reporter != nil { @@ -183,9 +188,9 @@ func (s *bitbucketIssueService) List(ctx context.Context, owner, repo string, op // Bitbucket uses q= query parameter for filtering switch opts.State { - case "open": + case stateOpen: url += "&q=state+%3D+%22new%22+OR+state+%3D+%22open%22" - case "closed": + case stateClosed: url += "&q=state+%3D+%22resolved%22+OR+state+%3D+%22closed%22" } @@ -263,7 +268,7 @@ func (s *bitbucketIssueService) Close(ctx context.Context, owner, repo string, n } func (s *bitbucketIssueService) Reopen(ctx context.Context, owner, repo string, number int) error { - body := map[string]any{"state": "open"} + body := map[string]any{"state": stateOpen} url := fmt.Sprintf("%s/repositories/%s/%s/issues/%d", bitbucketAPI, owner, repo, number) return s.doJSON(ctx, http.MethodPut, url, body, nil) } diff --git a/bitbucket/prs.go b/bitbucket/prs.go index 9b5dee5..962a282 100644 --- a/bitbucket/prs.go +++ b/bitbucket/prs.go @@ -10,6 +10,8 @@ import ( forge "github.com/git-pkgs/forge" ) +const maxDiffSize = 10 << 20 // 10 MB + type bitbucketPRService struct { token string httpClient *http.Client @@ -290,7 +292,7 @@ func (s *bitbucketPRService) Diff(ctx context.Context, owner, repo string, numbe return "", forge.ErrNotFound } - body, err := io.ReadAll(io.LimitReader(resp.Body, 10<<20)) // 10 MB max + body, err := io.ReadAll(io.LimitReader(resp.Body, maxDiffSize)) if err != nil { return "", err } diff --git a/forge.go b/forge.go index a29dcd8..895445b 100644 --- a/forge.go +++ b/forge.go @@ -241,8 +241,8 @@ func ParseRepoURL(rawURL string) (domain, owner, repo string, err error) { } // Handle git@ SSH URLs: git@github.com:owner/repo.git - if strings.HasPrefix(rawURL, "git@") { - rawURL = strings.TrimPrefix(rawURL, "git@") + if after, found := strings.CutPrefix(rawURL, "git@"); found { + rawURL = after colonIdx := strings.Index(rawURL, ":") if colonIdx < 0 { return "", "", "", fmt.Errorf("invalid SSH URL: missing colon") @@ -265,11 +265,13 @@ func ParseRepoURL(rawURL string) (domain, owner, repo string, err error) { return splitOwnerRepo(domain, u.Path) } +const minOwnerRepoParts = 2 + func splitOwnerRepo(domain, path string) (string, string, string, error) { path = strings.TrimSuffix(path, ".git") path = strings.Trim(path, "/") parts := strings.Split(path, "/") - if len(parts) < 2 { + if len(parts) < minOwnerRepoParts { return "", "", "", fmt.Errorf("URL path must contain owner/repo, got %q", path) } return domain, parts[0], parts[1], nil diff --git a/gitea/commit_statuses.go b/gitea/commit_statuses.go index be920a9..379b10f 100644 --- a/gitea/commit_statuses.go +++ b/gitea/commit_statuses.go @@ -8,6 +8,8 @@ import ( "code.gitea.io/sdk/gitea" ) +const defaultPageSize = 50 + type giteaCommitStatusService struct { client *gitea.Client } @@ -21,7 +23,7 @@ func (s *giteaCommitStatusService) List(ctx context.Context, owner, repo, sha st page := 1 for { statuses, resp, err := s.client.ListStatuses(owner, repo, sha, gitea.ListStatusesOption{ - ListOptions: gitea.ListOptions{Page: page, PageSize: 50}, + ListOptions: gitea.ListOptions{Page: page, PageSize: defaultPageSize}, }) if err != nil { if resp != nil && resp.StatusCode == http.StatusNotFound { @@ -42,7 +44,7 @@ func (s *giteaCommitStatusService) List(ctx context.Context, owner, repo, sha st } all = append(all, cs) } - if len(statuses) < 50 { + if len(statuses) < defaultPageSize { break } page++ diff --git a/gitea/gitea.go b/gitea/gitea.go index 102c40e..f87a401 100644 --- a/gitea/gitea.go +++ b/gitea/gitea.go @@ -96,7 +96,7 @@ func (s *giteaRepoService) Get(ctx context.Context, owner, repo string) (*forge. func (s *giteaRepoService) List(ctx context.Context, owner string, opts forge.ListRepoOpts) ([]forge.Repository, error) { perPage := opts.PerPage if perPage <= 0 { - perPage = 50 + perPage = defaultPageSize } // Try org endpoint first, fall back to user on 404. @@ -296,7 +296,7 @@ func (s *giteaRepoService) Fork(ctx context.Context, owner, repo string, opts fo func (s *giteaRepoService) ListForks(ctx context.Context, owner, repo string, opts forge.ListForksOpts) ([]forge.Repository, error) { perPage := opts.PerPage if perPage <= 0 { - perPage = 50 + perPage = defaultPageSize } page := opts.Page if page <= 0 { @@ -335,7 +335,7 @@ func (s *giteaRepoService) ListTags(ctx context.Context, owner, repo string) ([] page := 1 for { tags, resp, err := s.client.ListRepoTags(owner, repo, gitea.ListRepoTagsOptions{ - ListOptions: gitea.ListOptions{Page: page, PageSize: 50}, + ListOptions: gitea.ListOptions{Page: page, PageSize: defaultPageSize}, }) if err != nil { if resp != nil && resp.StatusCode == http.StatusNotFound { @@ -350,7 +350,7 @@ func (s *giteaRepoService) ListTags(ctx context.Context, owner, repo string) ([] } allTags = append(allTags, tag) } - if len(tags) < 50 { + if len(tags) < defaultPageSize { break } page++ diff --git a/gitea/issues.go b/gitea/issues.go index d0f9697..ebd81fd 100644 --- a/gitea/issues.go +++ b/gitea/issues.go @@ -28,9 +28,9 @@ func convertGiteaIssue(i *gitea.Issue) forge.Issue { switch i.State { case gitea.StateOpen: - result.State = "open" + result.State = stateOpen case gitea.StateClosed: - result.State = "closed" + result.State = stateClosed default: result.State = string(i.State) } @@ -134,11 +134,11 @@ func (s *giteaIssueService) List(ctx context.Context, owner, repo string, opts f } switch opts.State { - case "open": + case stateOpen: gOpts.State = gitea.StateOpen - case "closed": + case stateClosed: gOpts.State = gitea.StateClosed - case "all": + case stateAll: gOpts.State = gitea.StateAll default: gOpts.State = gitea.StateOpen @@ -296,7 +296,7 @@ func (s *giteaIssueService) ListComments(ctx context.Context, owner, repo string page := 1 for { comments, resp, err := s.client.ListIssueComments(owner, repo, int64(number), gitea.ListIssueCommentOptions{ - ListOptions: gitea.ListOptions{Page: page, PageSize: 50}, + ListOptions: gitea.ListOptions{Page: page, PageSize: defaultPageSize}, }) if err != nil { if resp != nil && resp.StatusCode == http.StatusNotFound { @@ -307,7 +307,7 @@ func (s *giteaIssueService) ListComments(ctx context.Context, owner, repo string for _, c := range comments { all = append(all, convertGiteaComment(c)) } - if len(comments) < 50 { + if len(comments) < defaultPageSize { break } page++ diff --git a/gitea/labels.go b/gitea/labels.go index f3dbc7f..2f4e9fb 100644 --- a/gitea/labels.go +++ b/gitea/labels.go @@ -29,7 +29,7 @@ func convertGiteaLabel(l *gitea.Label) forge.Label { func (s *giteaLabelService) List(ctx context.Context, owner, repo string, opts forge.ListLabelOpts) ([]forge.Label, error) { perPage := opts.PerPage if perPage <= 0 { - perPage = 50 + perPage = defaultPageSize } page := opts.Page if page <= 0 { @@ -69,7 +69,7 @@ func (s *giteaLabelService) findLabelByName(owner, repo, name string) (*gitea.La page := 1 for { labels, resp, err := s.client.ListRepoLabels(owner, repo, gitea.ListLabelsOptions{ - ListOptions: gitea.ListOptions{Page: page, PageSize: 50}, + ListOptions: gitea.ListOptions{Page: page, PageSize: defaultPageSize}, }) if err != nil { if resp != nil && resp.StatusCode == http.StatusNotFound { @@ -82,7 +82,7 @@ func (s *giteaLabelService) findLabelByName(owner, repo, name string) (*gitea.La return l, nil } } - if len(labels) < 50 { + if len(labels) < defaultPageSize { break } page++ @@ -102,7 +102,7 @@ func resolveLabelIDs(client *gitea.Client, owner, repo string, names []string) ( page := 1 for { labels, resp, err := client.ListRepoLabels(owner, repo, gitea.ListLabelsOptions{ - ListOptions: gitea.ListOptions{Page: page, PageSize: 50}, + ListOptions: gitea.ListOptions{Page: page, PageSize: defaultPageSize}, }) if err != nil { if resp != nil && resp.StatusCode == http.StatusNotFound { @@ -116,7 +116,7 @@ func resolveLabelIDs(client *gitea.Client, owner, repo string, names []string) ( delete(nameSet, l.Name) } } - if len(nameSet) == 0 || len(labels) < 50 { + if len(nameSet) == 0 || len(labels) < defaultPageSize { break } page++ diff --git a/gitea/milestones.go b/gitea/milestones.go index ba89dda..107b64f 100644 --- a/gitea/milestones.go +++ b/gitea/milestones.go @@ -8,6 +8,12 @@ import ( "code.gitea.io/sdk/gitea" ) +const ( + stateOpen = "open" + stateClosed = "closed" + stateAll = "all" +) + type giteaMilestoneService struct { client *gitea.Client } @@ -45,11 +51,11 @@ func (s *giteaMilestoneService) List(ctx context.Context, owner, repo string, op } switch opts.State { - case "open": + case stateOpen: gOpts.State = gitea.StateOpen - case "closed": + case stateClosed: gOpts.State = gitea.StateClosed - case "all": + case stateAll: gOpts.State = gitea.StateAll default: gOpts.State = gitea.StateOpen @@ -126,10 +132,10 @@ func (s *giteaMilestoneService) Update(ctx context.Context, owner, repo string, } if opts.State != nil { switch *opts.State { - case "open": + case stateOpen: s := gitea.StateOpen gOpts.State = &s - case "closed": + case stateClosed: s := gitea.StateClosed gOpts.State = &s } diff --git a/gitea/notifications.go b/gitea/notifications.go index 37765cc..a1ad05f 100644 --- a/gitea/notifications.go +++ b/gitea/notifications.go @@ -10,6 +10,8 @@ import ( forge "github.com/git-pkgs/forge" ) +const ownerRepoParts = 2 + type giteaNotificationService struct { client *gitea.Client } @@ -72,47 +74,18 @@ func (s *giteaNotificationService) List(ctx context.Context, opts forge.ListNoti } var all []forge.Notification + var err error if opts.Repo != "" { - parts := strings.SplitN(opts.Repo, "/", 2) - if len(parts) == 2 { - for { - notifications, resp, err := s.client.ListRepoNotifications(parts[0], parts[1], gitea.ListNotificationOptions{ - ListOptions: gitea.ListOptions{Page: page, PageSize: perPage}, - Status: statuses, - }) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - return nil, forge.ErrNotFound - } - return nil, err - } - for _, n := range notifications { - all = append(all, convertGiteaNotification(n)) - } - if len(notifications) < perPage || (opts.Limit > 0 && len(all) >= opts.Limit) { - break - } - page++ - } + parts := strings.SplitN(opts.Repo, "/", ownerRepoParts) + if len(parts) == ownerRepoParts { + all, err = s.listRepoNotifications(parts[0], parts[1], page, perPage, statuses, opts.Limit) } } else { - for { - notifications, _, err := s.client.ListNotifications(gitea.ListNotificationOptions{ - ListOptions: gitea.ListOptions{Page: page, PageSize: perPage}, - Status: statuses, - }) - if err != nil { - return nil, err - } - for _, n := range notifications { - all = append(all, convertGiteaNotification(n)) - } - if len(notifications) < perPage || (opts.Limit > 0 && len(all) >= opts.Limit) { - break - } - page++ - } + all, err = s.listAllNotifications(page, perPage, statuses, opts.Limit) + } + if err != nil { + return nil, err } if opts.Limit > 0 && len(all) > opts.Limit { @@ -122,6 +95,51 @@ func (s *giteaNotificationService) List(ctx context.Context, opts forge.ListNoti return all, nil } +func (s *giteaNotificationService) listRepoNotifications(owner, repo string, page, perPage int, statuses []gitea.NotifyStatus, limit int) ([]forge.Notification, error) { + var all []forge.Notification + for { + notifications, resp, err := s.client.ListRepoNotifications(owner, repo, gitea.ListNotificationOptions{ + ListOptions: gitea.ListOptions{Page: page, PageSize: perPage}, + Status: statuses, + }) + if err != nil { + if resp != nil && resp.StatusCode == http.StatusNotFound { + return nil, forge.ErrNotFound + } + return nil, err + } + for _, n := range notifications { + all = append(all, convertGiteaNotification(n)) + } + if len(notifications) < perPage || (limit > 0 && len(all) >= limit) { + break + } + page++ + } + return all, nil +} + +func (s *giteaNotificationService) listAllNotifications(page, perPage int, statuses []gitea.NotifyStatus, limit int) ([]forge.Notification, error) { + var all []forge.Notification + for { + notifications, _, err := s.client.ListNotifications(gitea.ListNotificationOptions{ + ListOptions: gitea.ListOptions{Page: page, PageSize: perPage}, + Status: statuses, + }) + if err != nil { + return nil, err + } + for _, n := range notifications { + all = append(all, convertGiteaNotification(n)) + } + if len(notifications) < perPage || (limit > 0 && len(all) >= limit) { + break + } + page++ + } + return all, nil +} + func (s *giteaNotificationService) MarkRead(ctx context.Context, opts forge.MarkNotificationOpts) error { if opts.ID != "" { id, err := strconv.ParseInt(opts.ID, 10, 64) @@ -139,8 +157,8 @@ func (s *giteaNotificationService) MarkRead(ctx context.Context, opts forge.Mark } if opts.Repo != "" { - parts := strings.SplitN(opts.Repo, "/", 2) - if len(parts) == 2 { + parts := strings.SplitN(opts.Repo, "/", ownerRepoParts) + if len(parts) == ownerRepoParts { _, resp, err := s.client.ReadRepoNotifications(parts[0], parts[1], gitea.MarkNotificationOptions{}) if err != nil { if resp != nil && resp.StatusCode == http.StatusNotFound { diff --git a/gitea/prs.go b/gitea/prs.go index 93dd835..45b5261 100644 --- a/gitea/prs.go +++ b/gitea/prs.go @@ -39,12 +39,13 @@ func convertGiteaPR(pr *gitea.PullRequest) forge.PullRequest { DiffURL: pr.DiffURL, } - if pr.HasMerged { + switch { + case pr.HasMerged: result.State = "merged" - } else if pr.State == gitea.StateClosed { - result.State = "closed" - } else { - result.State = "open" + case pr.State == gitea.StateClosed: + result.State = stateClosed + default: + result.State = stateOpen } if pr.Head != nil { @@ -135,11 +136,11 @@ func (s *giteaPRService) List(ctx context.Context, owner, repo string, opts forg } switch opts.State { - case "open": + case stateOpen: gOpts.State = gitea.StateOpen - case "closed": + case stateClosed: gOpts.State = gitea.StateClosed - case "all": + case stateAll: gOpts.State = gitea.StateAll default: gOpts.State = gitea.StateOpen @@ -322,7 +323,7 @@ func (s *giteaPRService) ListComments(ctx context.Context, owner, repo string, n page := 1 for { comments, resp, err := s.client.ListIssueComments(owner, repo, int64(number), gitea.ListIssueCommentOptions{ - ListOptions: gitea.ListOptions{Page: page, PageSize: 50}, + ListOptions: gitea.ListOptions{Page: page, PageSize: defaultPageSize}, }) if err != nil { if resp != nil && resp.StatusCode == http.StatusNotFound { @@ -333,7 +334,7 @@ func (s *giteaPRService) ListComments(ctx context.Context, owner, repo string, n for _, c := range comments { all = append(all, convertGiteaComment(c)) } - if len(comments) < 50 { + if len(comments) < defaultPageSize { break } page++ diff --git a/gitea/rate_limit.go b/gitea/rate_limit.go index 1117f8b..9eae28c 100644 --- a/gitea/rate_limit.go +++ b/gitea/rate_limit.go @@ -44,7 +44,7 @@ func (f *giteaForge) GetRateLimit(ctx context.Context) (*forge.RateLimit, error) if resp.StatusCode == http.StatusNotFound { return nil, fmt.Errorf("getting rate limit: %w", forge.ErrNotSupported) } - if resp.StatusCode >= 400 { + if resp.StatusCode >= http.StatusBadRequest { return nil, &forge.HTTPError{StatusCode: resp.StatusCode, URL: url} } diff --git a/gitea/releases.go b/gitea/releases.go index 69ddd98..6f0fde7 100644 --- a/gitea/releases.go +++ b/gitea/releases.go @@ -11,6 +11,8 @@ import ( "code.gitea.io/sdk/gitea" ) +const latestReleasesPageSize = 10 + type giteaReleaseService struct { client *gitea.Client } @@ -106,7 +108,7 @@ func (s *giteaReleaseService) Get(ctx context.Context, owner, repo, tag string) func (s *giteaReleaseService) GetLatest(ctx context.Context, owner, repo string) (*forge.Release, error) { // List releases and find the first non-draft, non-prerelease releases, resp, err := s.client.ListReleases(owner, repo, gitea.ListReleasesOptions{ - ListOptions: gitea.ListOptions{Page: 1, PageSize: 10}, + ListOptions: gitea.ListOptions{Page: 1, PageSize: latestReleasesPageSize}, }) if err != nil { if resp != nil && resp.StatusCode == http.StatusNotFound { diff --git a/github/ci.go b/github/ci.go index 483588a..2bf6513 100644 --- a/github/ci.go +++ b/github/ci.go @@ -9,6 +9,8 @@ import ( "github.com/google/go-github/v82/github" ) +const maxLogRedirects = 4 + type gitHubCIService struct { client *github.Client } @@ -192,7 +194,7 @@ func (s *gitHubCIService) RetryRun(ctx context.Context, owner, repo string, runI } func (s *gitHubCIService) GetJobLog(ctx context.Context, owner, repo string, jobID int64) (io.ReadCloser, error) { - url, resp, err := s.client.Actions.GetWorkflowJobLogs(ctx, owner, repo, jobID, 4) + url, resp, err := s.client.Actions.GetWorkflowJobLogs(ctx, owner, repo, jobID, maxLogRedirects) if err != nil { if resp != nil && resp.StatusCode == http.StatusNotFound { return nil, forge.ErrNotFound diff --git a/github/collaborators.go b/github/collaborators.go index 91c4781..12e6109 100644 --- a/github/collaborators.go +++ b/github/collaborators.go @@ -8,6 +8,12 @@ import ( "github.com/google/go-github/v82/github" ) +const ( + permRead = "read" + permWrite = "write" + permAdmin = "admin" +) + type gitHubCollaboratorService struct { client *github.Client } @@ -17,22 +23,22 @@ func (f *gitHubForge) Collaborators() forge.CollaboratorService { } func convertGitHubCollaborator(u *github.User) forge.Collaborator { - perm := "read" + perm := permRead if u.Permissions != nil { - if u.Permissions["admin"] { - perm = "admin" + if u.Permissions[permAdmin] { + perm = permAdmin } else if u.Permissions["push"] { - perm = "write" + perm = permWrite } } if u.RoleName != nil { switch u.GetRoleName() { - case "admin": - perm = "admin" - case "write": - perm = "write" - case "read": - perm = "read" + case permAdmin: + perm = permAdmin + case permWrite: + perm = permWrite + case permRead: + perm = permRead } } return forge.Collaborator{ diff --git a/github/commit_statuses.go b/github/commit_statuses.go index eff398e..a141e6b 100644 --- a/github/commit_statuses.go +++ b/github/commit_statuses.go @@ -18,7 +18,7 @@ func (f *gitHubForge) CommitStatuses() forge.CommitStatusService { func (s *gitHubCommitStatusService) List(ctx context.Context, owner, repo, sha string) ([]forge.CommitStatus, error) { var all []forge.CommitStatus - opts := &github.ListOptions{PerPage: 100} + opts := &github.ListOptions{PerPage: defaultPageSize} for { statuses, resp, err := s.client.Repositories.ListStatuses(ctx, owner, repo, sha, opts) if err != nil { diff --git a/github/github.go b/github/github.go index 3fac40d..5015729 100644 --- a/github/github.go +++ b/github/github.go @@ -8,6 +8,8 @@ import ( "github.com/google/go-github/v82/github" ) +const defaultPageSize = 100 + type gitHubForge struct { client *github.Client } @@ -103,7 +105,7 @@ func (s *gitHubRepoService) Get(ctx context.Context, owner, repo string) (*forge func (s *gitHubRepoService) List(ctx context.Context, owner string, opts forge.ListRepoOpts) ([]forge.Repository, error) { perPage := opts.PerPage if perPage <= 0 { - perPage = 100 + perPage = defaultPageSize } // Try org endpoint first, fall back to user on 404. @@ -305,7 +307,7 @@ func (s *gitHubRepoService) Fork(ctx context.Context, owner, repo string, opts f func (s *gitHubRepoService) ListForks(ctx context.Context, owner, repo string, opts forge.ListForksOpts) ([]forge.Repository, error) { perPage := opts.PerPage if perPage <= 0 { - perPage = 100 + perPage = defaultPageSize } page := opts.Page if page <= 0 { @@ -344,7 +346,7 @@ func (s *gitHubRepoService) ListForks(ctx context.Context, owner, repo string, o func (s *gitHubRepoService) ListTags(ctx context.Context, owner, repo string) ([]forge.Tag, error) { var allTags []forge.Tag - opts := &github.ListOptions{PerPage: 100} + opts := &github.ListOptions{PerPage: defaultPageSize} for { tags, resp, err := s.client.Repositories.ListTags(ctx, owner, repo, opts) if err != nil { @@ -371,7 +373,7 @@ func (s *gitHubRepoService) ListTags(ctx context.Context, owner, repo string) ([ func (s *gitHubRepoService) ListContributors(ctx context.Context, owner, repo string) ([]forge.Contributor, error) { var all []forge.Contributor opts := &github.ListContributorsOptions{ - ListOptions: github.ListOptions{PerPage: 100}, + ListOptions: github.ListOptions{PerPage: defaultPageSize}, } for { contributors, resp, err := s.client.Repositories.ListContributors(ctx, owner, repo, opts) diff --git a/github/issues.go b/github/issues.go index 1668207..d953ea6 100644 --- a/github/issues.go +++ b/github/issues.go @@ -9,6 +9,11 @@ import ( "github.com/google/go-github/v82/github" ) +const ( + stateOpen = "open" + stateClosed = "closed" +) + type gitHubIssueService struct { client *github.Client } @@ -240,7 +245,7 @@ func (s *gitHubIssueService) Update(ctx context.Context, owner, repo string, num } func (s *gitHubIssueService) Close(ctx context.Context, owner, repo string, number int) error { - state := "closed" + state := stateClosed req := &github.IssueRequest{State: &state} _, resp, err := s.client.Issues.Edit(ctx, owner, repo, number, req) if err != nil { @@ -253,7 +258,7 @@ func (s *gitHubIssueService) Close(ctx context.Context, owner, repo string, numb } func (s *gitHubIssueService) Reopen(ctx context.Context, owner, repo string, number int) error { - state := "open" + state := stateOpen req := &github.IssueRequest{State: &state} _, resp, err := s.client.Issues.Edit(ctx, owner, repo, number, req) if err != nil { @@ -287,7 +292,7 @@ func (s *gitHubIssueService) CreateComment(ctx context.Context, owner, repo stri func (s *gitHubIssueService) ListComments(ctx context.Context, owner, repo string, number int) ([]forge.Comment, error) { var all []forge.Comment ghOpts := &github.IssueListCommentsOptions{ - ListOptions: github.ListOptions{PerPage: 100}, + ListOptions: github.ListOptions{PerPage: defaultPageSize}, } for { comments, resp, err := s.client.Issues.ListComments(ctx, owner, repo, number, ghOpts) diff --git a/github/milestones.go b/github/milestones.go index a45ce2a..44a6f18 100644 --- a/github/milestones.go +++ b/github/milestones.go @@ -139,7 +139,7 @@ func (s *gitHubMilestoneService) Update(ctx context.Context, owner, repo string, } func (s *gitHubMilestoneService) Close(ctx context.Context, owner, repo string, id int) error { - state := "closed" + state := stateClosed ghMilestone := &github.Milestone{State: &state} _, resp, err := s.client.Issues.EditMilestone(ctx, owner, repo, id, ghMilestone) if err != nil { @@ -152,7 +152,7 @@ func (s *gitHubMilestoneService) Close(ctx context.Context, owner, repo string, } func (s *gitHubMilestoneService) Reopen(ctx context.Context, owner, repo string, id int) error { - state := "open" + state := stateOpen ghMilestone := &github.Milestone{State: &state} _, resp, err := s.client.Issues.EditMilestone(ctx, owner, repo, id, ghMilestone) if err != nil { diff --git a/github/notifications.go b/github/notifications.go index 0b0b026..4118f88 100644 --- a/github/notifications.go +++ b/github/notifications.go @@ -9,6 +9,8 @@ import ( "github.com/google/go-github/v82/github" ) +const ownerRepoParts = 2 + type gitHubNotificationService struct { client *github.Client } @@ -76,41 +78,18 @@ func (s *gitHubNotificationService) List(ctx context.Context, opts forge.ListNot } var all []forge.Notification + var err error if opts.Repo != "" { - parts := strings.SplitN(opts.Repo, "/", 2) - if len(parts) == 2 { - for { - notifications, resp, err := s.client.Activity.ListRepositoryNotifications(ctx, parts[0], parts[1], ghOpts) - if err != nil { - if resp != nil && resp.StatusCode == http.StatusNotFound { - return nil, forge.ErrNotFound - } - return nil, err - } - for _, n := range notifications { - all = append(all, convertGitHubNotification(n)) - } - if resp.NextPage == 0 || (opts.Limit > 0 && len(all) >= opts.Limit) { - break - } - ghOpts.Page = resp.NextPage - } + parts := strings.SplitN(opts.Repo, "/", ownerRepoParts) + if len(parts) == ownerRepoParts { + all, err = s.listRepoNotifications(ctx, parts[0], parts[1], ghOpts, opts.Limit) } } else { - for { - notifications, resp, err := s.client.Activity.ListNotifications(ctx, ghOpts) - if err != nil { - return nil, err - } - for _, n := range notifications { - all = append(all, convertGitHubNotification(n)) - } - if resp.NextPage == 0 || (opts.Limit > 0 && len(all) >= opts.Limit) { - break - } - ghOpts.Page = resp.NextPage - } + all, err = s.listAllNotifications(ctx, ghOpts, opts.Limit) + } + if err != nil { + return nil, err } if opts.Limit > 0 && len(all) > opts.Limit { @@ -120,6 +99,45 @@ func (s *gitHubNotificationService) List(ctx context.Context, opts forge.ListNot return all, nil } +func (s *gitHubNotificationService) listRepoNotifications(ctx context.Context, owner, repo string, ghOpts *github.NotificationListOptions, limit int) ([]forge.Notification, error) { + var all []forge.Notification + for { + notifications, resp, err := s.client.Activity.ListRepositoryNotifications(ctx, owner, repo, ghOpts) + if err != nil { + if resp != nil && resp.StatusCode == http.StatusNotFound { + return nil, forge.ErrNotFound + } + return nil, err + } + for _, n := range notifications { + all = append(all, convertGitHubNotification(n)) + } + if resp.NextPage == 0 || (limit > 0 && len(all) >= limit) { + break + } + ghOpts.Page = resp.NextPage + } + return all, nil +} + +func (s *gitHubNotificationService) listAllNotifications(ctx context.Context, ghOpts *github.NotificationListOptions, limit int) ([]forge.Notification, error) { + var all []forge.Notification + for { + notifications, resp, err := s.client.Activity.ListNotifications(ctx, ghOpts) + if err != nil { + return nil, err + } + for _, n := range notifications { + all = append(all, convertGitHubNotification(n)) + } + if resp.NextPage == 0 || (limit > 0 && len(all) >= limit) { + break + } + ghOpts.Page = resp.NextPage + } + return all, nil +} + func (s *gitHubNotificationService) MarkRead(ctx context.Context, opts forge.MarkNotificationOpts) error { if opts.ID != "" { resp, err := s.client.Activity.MarkThreadRead(ctx, opts.ID) @@ -133,8 +151,8 @@ func (s *gitHubNotificationService) MarkRead(ctx context.Context, opts forge.Mar } if opts.Repo != "" { - parts := strings.SplitN(opts.Repo, "/", 2) - if len(parts) == 2 { + parts := strings.SplitN(opts.Repo, "/", ownerRepoParts) + if len(parts) == ownerRepoParts { resp, err := s.client.Activity.MarkRepositoryNotificationsRead(ctx, parts[0], parts[1], github.Timestamp{}) if err != nil { if resp != nil && resp.StatusCode == http.StatusNotFound { diff --git a/github/prs.go b/github/prs.go index ac71cdd..16161df 100644 --- a/github/prs.go +++ b/github/prs.go @@ -145,7 +145,7 @@ func (s *gitHubPRService) List(ctx context.Context, owner, repo string, opts for ListOptions: github.ListOptions{PerPage: perPage, Page: page}, } if ghOpts.State == "" { - ghOpts.State = "open" + ghOpts.State = stateOpen } var all []forge.PullRequest @@ -262,7 +262,7 @@ func (s *gitHubPRService) Update(ctx context.Context, owner, repo string, number } func (s *gitHubPRService) Close(ctx context.Context, owner, repo string, number int) error { - state := "closed" + state := stateClosed _, resp, err := s.client.PullRequests.Edit(ctx, owner, repo, number, &github.PullRequest{ State: &state, }) @@ -276,7 +276,7 @@ func (s *gitHubPRService) Close(ctx context.Context, owner, repo string, number } func (s *gitHubPRService) Reopen(ctx context.Context, owner, repo string, number int) error { - state := "open" + state := stateOpen _, resp, err := s.client.PullRequests.Edit(ctx, owner, repo, number, &github.PullRequest{ State: &state, }) @@ -352,7 +352,7 @@ func (s *gitHubPRService) CreateComment(ctx context.Context, owner, repo string, func (s *gitHubPRService) ListComments(ctx context.Context, owner, repo string, number int) ([]forge.Comment, error) { var all []forge.Comment ghOpts := &github.IssueListCommentsOptions{ - ListOptions: github.ListOptions{PerPage: 100}, + ListOptions: github.ListOptions{PerPage: defaultPageSize}, } for { comments, resp, err := s.client.Issues.ListComments(ctx, owner, repo, number, ghOpts) diff --git a/github/reactions.go b/github/reactions.go index cae1707..ffe0e53 100644 --- a/github/reactions.go +++ b/github/reactions.go @@ -22,7 +22,7 @@ func convertGitHubReaction(r *github.Reaction) forge.Reaction { func (s *gitHubIssueService) ListReactions(ctx context.Context, owner, repo string, number int, commentID int64) ([]forge.Reaction, error) { var all []forge.Reaction opts := &github.ListReactionOptions{ - ListOptions: github.ListOptions{PerPage: 100}, + ListOptions: github.ListOptions{PerPage: defaultPageSize}, } for { reactions, resp, err := s.client.Reactions.ListIssueCommentReactions(ctx, owner, repo, commentID, opts) @@ -59,7 +59,7 @@ func (s *gitHubPRService) ListReactions(ctx context.Context, owner, repo string, // GitHub uses the same issue comment reactions API for PR comments var all []forge.Reaction opts := &github.ListReactionOptions{ - ListOptions: github.ListOptions{PerPage: 100}, + ListOptions: github.ListOptions{PerPage: defaultPageSize}, } for { reactions, resp, err := s.client.Reactions.ListIssueCommentReactions(ctx, owner, repo, commentID, opts) diff --git a/github/secrets.go b/github/secrets.go index ece4fdd..7d193e2 100644 --- a/github/secrets.go +++ b/github/secrets.go @@ -12,6 +12,8 @@ import ( "golang.org/x/crypto/nacl/box" ) +const naclKeySize = 32 + type gitHubSecretService struct { client *github.Client } @@ -106,8 +108,8 @@ func encryptSecret(publicKeyB64, secretValue string) (string, error) { } var recipientKey [32]byte - if len(publicKeyBytes) != 32 { - return "", fmt.Errorf("public key must be 32 bytes, got %d", len(publicKeyBytes)) + if len(publicKeyBytes) != naclKeySize { + return "", fmt.Errorf("public key must be %d bytes, got %d", naclKeySize, len(publicKeyBytes)) } copy(recipientKey[:], publicKeyBytes) diff --git a/gitlab/commit_statuses.go b/gitlab/commit_statuses.go index 2eb97a6..8643ed8 100644 --- a/gitlab/commit_statuses.go +++ b/gitlab/commit_statuses.go @@ -8,6 +8,8 @@ import ( gitlab "gitlab.com/gitlab-org/api/client-go" ) +const defaultPageSize = 100 + type gitLabCommitStatusService struct { client *gitlab.Client } @@ -20,7 +22,7 @@ func (s *gitLabCommitStatusService) List(ctx context.Context, owner, repo, sha s pid := owner + "/" + repo var all []forge.CommitStatus opts := &gitlab.GetCommitStatusesOptions{ - ListOptions: gitlab.ListOptions{PerPage: 100}, + ListOptions: gitlab.ListOptions{PerPage: defaultPageSize}, } for { statuses, resp, err := s.client.Commits.GetCommitStatuses(pid, sha, opts) diff --git a/gitlab/gitlab.go b/gitlab/gitlab.go index 800f012..bea05a8 100644 --- a/gitlab/gitlab.go +++ b/gitlab/gitlab.go @@ -110,7 +110,7 @@ func (s *gitLabRepoService) List(ctx context.Context, owner string, opts forge.L return forge.FilterRepos(repos, opts), nil } -func (s *gitLabRepoService) listGroupProjects(ctx context.Context, group string, perPage int) ([]forge.Repository, error) { +func (s *gitLabRepoService) listGroupProjects(_ context.Context, group string, perPage int) ([]forge.Repository, error) { var all []forge.Repository glOpts := &gitlab.ListGroupProjectsOptions{ ListOptions: gitlab.ListOptions{PerPage: int64(perPage)}, @@ -134,7 +134,7 @@ func (s *gitLabRepoService) listGroupProjects(ctx context.Context, group string, return all, nil } -func (s *gitLabRepoService) listUserProjects(ctx context.Context, user string, perPage int) ([]forge.Repository, error) { +func (s *gitLabRepoService) listUserProjects(_ context.Context, user string, perPage int) ([]forge.Repository, error) { var all []forge.Repository glOpts := &gitlab.ListProjectsOptions{ ListOptions: gitlab.ListOptions{PerPage: int64(perPage)}, @@ -331,7 +331,7 @@ func (s *gitLabRepoService) ListTags(ctx context.Context, owner, repo string) ([ pid := owner + "/" + repo var allTags []forge.Tag opts := &gitlab.ListTagsOptions{ - ListOptions: gitlab.ListOptions{PerPage: 100}, + ListOptions: gitlab.ListOptions{PerPage: defaultPageSize}, } for { tags, resp, err := s.client.Tags.ListTags(pid, opts) @@ -360,7 +360,7 @@ func (s *gitLabRepoService) ListContributors(ctx context.Context, owner, repo st pid := owner + "/" + repo var all []forge.Contributor opts := &gitlab.ListContributorsOptions{ - ListOptions: gitlab.ListOptions{PerPage: 100}, + ListOptions: gitlab.ListOptions{PerPage: defaultPageSize}, } for { contributors, resp, err := s.client.Repositories.Contributors(pid, opts) diff --git a/gitlab/issues.go b/gitlab/issues.go index d8d84ce..503a93f 100644 --- a/gitlab/issues.go +++ b/gitlab/issues.go @@ -10,6 +10,10 @@ import ( gitlab "gitlab.com/gitlab-org/api/client-go" ) +const ( + stateAll = "all" +) + type gitLabIssueService struct { client *gitlab.Client } @@ -126,7 +130,7 @@ func (s *gitLabIssueService) List(ctx context.Context, owner, repo string, opts ListOptions: gitlab.ListOptions{PerPage: int64(perPage), Page: int64(page)}, } - if opts.State != "" && opts.State != "all" { + if opts.State != "" && opts.State != stateAll { glOpts.State = gitlab.Ptr(opts.State) } if opts.Assignee != "" { @@ -291,7 +295,7 @@ func (s *gitLabIssueService) ListComments(ctx context.Context, owner, repo strin pid := owner + "/" + repo var all []forge.Comment glOpts := &gitlab.ListIssueNotesOptions{ - ListOptions: gitlab.ListOptions{PerPage: 100}, + ListOptions: gitlab.ListOptions{PerPage: defaultPageSize}, } for { notes, resp, err := s.client.Notes.ListIssueNotes(pid, int64(number), glOpts) diff --git a/gitlab/milestones.go b/gitlab/milestones.go index 00d5794..b27ca3c 100644 --- a/gitlab/milestones.go +++ b/gitlab/milestones.go @@ -9,6 +9,10 @@ import ( gitlab "gitlab.com/gitlab-org/api/client-go" ) +const ( + stateOpen = "open" +) + type gitLabMilestoneService struct { client *gitlab.Client } @@ -46,7 +50,7 @@ func (s *gitLabMilestoneService) List(ctx context.Context, owner, repo string, o ListOptions: gitlab.ListOptions{PerPage: int64(perPage), Page: int64(page)}, } - if opts.State != "" && opts.State != "all" { + if opts.State != "" && opts.State != stateAll { glOpts.State = gitlab.Ptr(opts.State) } @@ -128,7 +132,7 @@ func (s *gitLabMilestoneService) Update(ctx context.Context, owner, repo string, switch *opts.State { case "closed": glOpts.StateEvent = gitlab.Ptr("close") - case "open": + case stateOpen: glOpts.StateEvent = gitlab.Ptr("activate") } changed = true diff --git a/gitlab/notifications.go b/gitlab/notifications.go index d5d34f0..5e29ddc 100644 --- a/gitlab/notifications.go +++ b/gitlab/notifications.go @@ -144,7 +144,7 @@ func (s *gitLabNotificationService) Get(ctx context.Context, id string) (*forge. } todos, _, err := s.client.Todos.ListTodos(&gitlab.ListTodosOptions{ - ListOptions: gitlab.ListOptions{PerPage: 100}, + ListOptions: gitlab.ListOptions{PerPage: defaultPageSize}, }) if err != nil { return nil, err diff --git a/gitlab/prs.go b/gitlab/prs.go index 5082d73..cc2f0f2 100644 --- a/gitlab/prs.go +++ b/gitlab/prs.go @@ -9,6 +9,10 @@ import ( gitlab "gitlab.com/gitlab-org/api/client-go" ) +const ( + stateOpened = "opened" +) + type gitLabPRService struct { client *gitlab.Client } @@ -33,8 +37,8 @@ func convertGitLabMR(mr *gitlab.MergeRequest) forge.PullRequest { } // Normalize "opened" to "open" - if result.State == "opened" { - result.State = "open" + if result.State == stateOpened { + result.State = stateOpen } if mr.Author != nil { @@ -119,8 +123,8 @@ func convertBasicGitLabMR(mr *gitlab.BasicMergeRequest) forge.PullRequest { HTMLURL: mr.WebURL, } - if result.State == "opened" { - result.State = "open" + if result.State == stateOpened { + result.State = stateOpen } if mr.Author != nil { @@ -198,11 +202,11 @@ func (s *gitLabPRService) List(ctx context.Context, owner, repo string, opts for ListOptions: gitlab.ListOptions{PerPage: int64(perPage), Page: int64(page)}, } - if opts.State != "" && opts.State != "all" { + if opts.State != "" && opts.State != stateAll { // GitLab uses "opened" not "open" state := opts.State - if state == "open" { - state = "opened" + if state == stateOpen { + state = stateOpened } glOpts.State = gitlab.Ptr(state) } @@ -407,7 +411,7 @@ func (s *gitLabPRService) ListComments(ctx context.Context, owner, repo string, pid := owner + "/" + repo var all []forge.Comment glOpts := &gitlab.ListMergeRequestNotesOptions{ - ListOptions: gitlab.ListOptions{PerPage: 100}, + ListOptions: gitlab.ListOptions{PerPage: defaultPageSize}, } for { notes, resp, err := s.client.Notes.ListMergeRequestNotes(pid, int64(number), glOpts) diff --git a/internal/cli/api.go b/internal/cli/api.go index f6f290a..1b61959 100644 --- a/internal/cli/api.go +++ b/internal/cli/api.go @@ -13,6 +13,8 @@ import ( "github.com/spf13/cobra" ) +const maxErrorBodyLength = 200 + var apiCmd = &cobra.Command{ Use: "api ", Short: "Make an authenticated API request", @@ -112,10 +114,10 @@ var apiCmd = &cobra.Command{ } } - if resp.StatusCode >= 400 { + if resp.StatusCode >= http.StatusBadRequest { msg := strings.TrimSpace(string(respBody)) - if len(msg) > 200 { - msg = msg[:200] + if len(msg) > maxErrorBodyLength { + msg = msg[:maxErrorBodyLength] } if msg != "" { return fmt.Errorf("HTTP %d: %s", resp.StatusCode, msg) diff --git a/internal/cli/branch.go b/internal/cli/branch.go index 91c7583..3f3ec77 100644 --- a/internal/cli/branch.go +++ b/internal/cli/branch.go @@ -10,6 +10,8 @@ import ( "github.com/spf13/cobra" ) +const shortSHALength = 7 + var branchCmd = &cobra.Command{ Use: "branch", Short: "Manage branches", @@ -61,8 +63,8 @@ func branchListCmd() *cobra.Command { rows := make([][]string, len(branches)) for i, b := range branches { sha := b.SHA - if len(sha) > 7 { - sha = sha[:7] + if len(sha) > shortSHALength { + sha = sha[:shortSHALength] } rows[i] = []string{ b.Name, diff --git a/internal/cli/ci.go b/internal/cli/ci.go index 150a844..260138b 100644 --- a/internal/cli/ci.go +++ b/internal/cli/ci.go @@ -13,6 +13,8 @@ import ( "github.com/spf13/cobra" ) +const defaultCIRunLimit = 20 + var ciCmd = &cobra.Command{ Use: "ci", Short: "Manage CI/CD pipelines", @@ -102,7 +104,7 @@ func ciListCmd() *cobra.Command { cmd.Flags().StringVarP(&flagStatus, "status", "s", "", "Filter by status") cmd.Flags().StringVarP(&flagUser, "user", "u", "", "Filter by user") cmd.Flags().StringVar(&flagWorkflow, "workflow", "", "Filter by workflow name or file") - cmd.Flags().IntVarP(&flagLimit, "limit", "L", 20, "Maximum number of runs") + cmd.Flags().IntVarP(&flagLimit, "limit", "L", defaultCIRunLimit, "Maximum number of runs") return cmd } @@ -141,7 +143,7 @@ func ciViewCmd() *cobra.Command { _, _ = fmt.Fprintf(os.Stdout, "Status: %s\n", status) _, _ = fmt.Fprintf(os.Stdout, "Branch: %s\n", run.Branch) if run.SHA != "" { - _, _ = fmt.Fprintf(os.Stdout, "SHA: %s\n", run.SHA[:min(7, len(run.SHA))]) + _, _ = fmt.Fprintf(os.Stdout, "SHA: %s\n", run.SHA[:min(shortSHALength, len(run.SHA))]) } if run.HTMLURL != "" { _, _ = fmt.Fprintf(os.Stdout, "URL: %s\n", run.HTMLURL) diff --git a/internal/cli/issue.go b/internal/cli/issue.go index 0370347..e089f1d 100644 --- a/internal/cli/issue.go +++ b/internal/cli/issue.go @@ -12,6 +12,12 @@ import ( "github.com/spf13/cobra" ) +const ( + maxTitleLength = 60 + truncatedTitleLen = 57 + defaultIssueLimit = 30 +) + var issueCmd = &cobra.Command{ Use: "issue", Short: "Manage issues", @@ -165,8 +171,8 @@ func issueListCmd() *cobra.Command { labels[j] = l.Name } title := iss.Title - if len(title) > 60 { - title = title[:57] + "..." + if len(title) > maxTitleLength { + title = title[:truncatedTitleLen] + "..." } rows[i] = []string{ strconv.Itoa(iss.Number), @@ -185,7 +191,7 @@ func issueListCmd() *cobra.Command { cmd.Flags().StringVarP(&flagAssignee, "assignee", "a", "", "Filter by assignee") cmd.Flags().StringVarP(&flagAuthor, "author", "A", "", "Filter by author") cmd.Flags().StringSliceVarP(&flagLabels, "label", "l", nil, "Filter by label") - cmd.Flags().IntVarP(&flagLimit, "limit", "L", 30, "Maximum number of issues") + cmd.Flags().IntVarP(&flagLimit, "limit", "L", defaultIssueLimit, "Maximum number of issues") cmd.Flags().StringVar(&flagSort, "sort", "", "Sort by: created, updated, comments") cmd.Flags().StringVar(&flagOrder, "order", "", "Sort order: asc, desc") return cmd diff --git a/internal/cli/label.go b/internal/cli/label.go index 73fd933..7417bac 100644 --- a/internal/cli/label.go +++ b/internal/cli/label.go @@ -10,6 +10,8 @@ import ( "github.com/spf13/cobra" ) +const maxLabelDescLength = 50 + var labelCmd = &cobra.Command{ Use: "label", Short: "Manage labels", @@ -62,7 +64,7 @@ func labelListCmd() *cobra.Command { rows := make([][]string, len(labels)) for i, l := range labels { desc := l.Description - if len(desc) > 50 { + if len(desc) > maxLabelDescLength { desc = desc[:47] + "..." } rows[i] = []string{l.Name, l.Color, desc} diff --git a/internal/cli/milestone.go b/internal/cli/milestone.go index 9a80d52..692d81a 100644 --- a/internal/cli/milestone.go +++ b/internal/cli/milestone.go @@ -12,6 +12,8 @@ import ( "github.com/spf13/cobra" ) +const defaultMilestoneLimit = 30 + var milestoneCmd = &cobra.Command{ Use: "milestone", Short: "Manage milestones", @@ -87,7 +89,7 @@ func milestoneListCmd() *cobra.Command { } cmd.Flags().StringVarP(&flagState, "state", "s", "open", "Filter by state: open, closed, all") - cmd.Flags().IntVarP(&flagLimit, "limit", "L", 30, "Maximum number of milestones") + cmd.Flags().IntVarP(&flagLimit, "limit", "L", defaultMilestoneLimit, "Maximum number of milestones") return cmd } diff --git a/internal/cli/notification.go b/internal/cli/notification.go index ad19cba..1f1f9c9 100644 --- a/internal/cli/notification.go +++ b/internal/cli/notification.go @@ -10,6 +10,12 @@ import ( "github.com/spf13/cobra" ) +const ( + maxNotifTitleLength = 60 + truncatedNotifTitleLen = 57 + defaultNotifLimit = 30 +) + var notificationCmd = &cobra.Command{ Use: "notification", Short: "Manage notifications", @@ -80,8 +86,8 @@ func notificationListCmd() *cobra.Command { status = "unread" } title := n.Title - if len(title) > 60 { - title = title[:57] + "..." + if len(title) > maxNotifTitleLength { + title = title[:truncatedNotifTitleLen] + "..." } rows[i] = []string{ n.ID, @@ -98,7 +104,7 @@ func notificationListCmd() *cobra.Command { cmd.Flags().BoolVar(&flagUnread, "unread", false, "Only show unread notifications") cmd.Flags().StringVar(&flagNRepo, "repo", "", "Filter by repository (owner/repo)") - cmd.Flags().IntVarP(&flagLimit, "limit", "L", 30, "Maximum number of notifications") + cmd.Flags().IntVarP(&flagLimit, "limit", "L", defaultNotifLimit, "Maximum number of notifications") return cmd } diff --git a/internal/cli/pr.go b/internal/cli/pr.go index 2134123..314ae81 100644 --- a/internal/cli/pr.go +++ b/internal/cli/pr.go @@ -6,12 +6,17 @@ import ( "strconv" "strings" - "github.com/git-pkgs/forge" + forges "github.com/git-pkgs/forge" "github.com/git-pkgs/forge/internal/output" "github.com/git-pkgs/forge/internal/resolve" "github.com/spf13/cobra" ) +const ( + maxPRTitleLength = 60 + defaultPRLimit = 30 +) + var prCmd = &cobra.Command{ Use: "pr", Aliases: []string{"mr"}, @@ -61,43 +66,7 @@ func prViewCmd() *cobra.Command { return p.PrintJSON(pr) } - _, _ = fmt.Fprintf(os.Stdout, "#%d %s\n", pr.Number, pr.Title) - _, _ = fmt.Fprintf(os.Stdout, "State: %s\n", pr.State) - _, _ = fmt.Fprintf(os.Stdout, "Author: %s\n", pr.Author.Login) - _, _ = fmt.Fprintf(os.Stdout, "Branch: %s -> %s\n", pr.Head, pr.Base) - - if pr.Draft { - _, _ = fmt.Fprintln(os.Stdout, "Draft: yes") - } - - if len(pr.Reviewers) > 0 { - names := make([]string, len(pr.Reviewers)) - for i, r := range pr.Reviewers { - names[i] = r.Login - } - _, _ = fmt.Fprintf(os.Stdout, "Review: %s\n", strings.Join(names, ", ")) - } - - if len(pr.Labels) > 0 { - names := make([]string, len(pr.Labels)) - for i, l := range pr.Labels { - names[i] = l.Name - } - _, _ = fmt.Fprintf(os.Stdout, "Labels: %s\n", strings.Join(names, ", ")) - } - - if pr.Milestone != nil { - _, _ = fmt.Fprintf(os.Stdout, "Mile: %s\n", pr.Milestone.Title) - } - - if pr.Additions > 0 || pr.Deletions > 0 { - _, _ = fmt.Fprintf(os.Stdout, "Changes: +%d -%d (%d files)\n", pr.Additions, pr.Deletions, pr.ChangedFiles) - } - - if pr.Body != "" { - _, _ = fmt.Fprintln(os.Stdout) - _, _ = fmt.Fprintln(os.Stdout, pr.Body) - } + printPRDetails(pr) if flagComments { comments, err := forge.PullRequests().ListComments(cmd.Context(), owner, repoName, number) @@ -119,6 +88,46 @@ func prViewCmd() *cobra.Command { return cmd } +func printPRDetails(pr *forges.PullRequest) { + _, _ = fmt.Fprintf(os.Stdout, "#%d %s\n", pr.Number, pr.Title) + _, _ = fmt.Fprintf(os.Stdout, "State: %s\n", pr.State) + _, _ = fmt.Fprintf(os.Stdout, "Author: %s\n", pr.Author.Login) + _, _ = fmt.Fprintf(os.Stdout, "Branch: %s -> %s\n", pr.Head, pr.Base) + + if pr.Draft { + _, _ = fmt.Fprintln(os.Stdout, "Draft: yes") + } + + if len(pr.Reviewers) > 0 { + names := make([]string, len(pr.Reviewers)) + for i, r := range pr.Reviewers { + names[i] = r.Login + } + _, _ = fmt.Fprintf(os.Stdout, "Review: %s\n", strings.Join(names, ", ")) + } + + if len(pr.Labels) > 0 { + names := make([]string, len(pr.Labels)) + for i, l := range pr.Labels { + names[i] = l.Name + } + _, _ = fmt.Fprintf(os.Stdout, "Labels: %s\n", strings.Join(names, ", ")) + } + + if pr.Milestone != nil { + _, _ = fmt.Fprintf(os.Stdout, "Mile: %s\n", pr.Milestone.Title) + } + + if pr.Additions > 0 || pr.Deletions > 0 { + _, _ = fmt.Fprintf(os.Stdout, "Changes: +%d -%d (%d files)\n", pr.Additions, pr.Deletions, pr.ChangedFiles) + } + + if pr.Body != "" { + _, _ = fmt.Fprintln(os.Stdout) + _, _ = fmt.Fprintln(os.Stdout, pr.Body) + } +} + func prListCmd() *cobra.Command { var ( flagState string @@ -174,8 +183,8 @@ func prListCmd() *cobra.Command { rows := make([][]string, len(prs)) for i, pr := range prs { title := pr.Title - if len(title) > 60 { - title = title[:57] + "..." + if len(title) > maxPRTitleLength { + title = title[:maxPRTitleLength-3] + "..." } rows[i] = []string{ strconv.Itoa(pr.Number), @@ -195,7 +204,7 @@ func prListCmd() *cobra.Command { cmd.Flags().StringVar(&flagHead, "head", "", "Filter by head branch") cmd.Flags().StringVar(&flagBase, "base", "", "Filter by base branch") cmd.Flags().StringSliceVarP(&flagLabels, "label", "l", nil, "Filter by label") - cmd.Flags().IntVarP(&flagLimit, "limit", "L", 30, "Maximum number of PRs") + cmd.Flags().IntVarP(&flagLimit, "limit", "L", defaultPRLimit, "Maximum number of PRs") cmd.Flags().StringVar(&flagSort, "sort", "", "Sort by: created, updated") cmd.Flags().StringVar(&flagOrder, "order", "", "Sort order: asc, desc") return cmd diff --git a/internal/cli/release.go b/internal/cli/release.go index 6d6effa..10a92ae 100644 --- a/internal/cli/release.go +++ b/internal/cli/release.go @@ -10,6 +10,11 @@ import ( "github.com/spf13/cobra" ) +const ( + defaultReleaseLimit = 30 + releaseAssetArgs = 2 +) + var releaseCmd = &cobra.Command{ Use: "release", Short: "Manage releases", @@ -81,7 +86,7 @@ func releaseListCmd() *cobra.Command { }, } - cmd.Flags().IntVarP(&flagLimit, "limit", "L", 30, "Maximum number of releases") + cmd.Flags().IntVarP(&flagLimit, "limit", "L", defaultReleaseLimit, "Maximum number of releases") return cmd } @@ -311,7 +316,7 @@ func releaseUploadCmd() *cobra.Command { return &cobra.Command{ Use: "upload ", Short: "Upload an asset to a release", - Args: cobra.ExactArgs(2), + Args: cobra.ExactArgs(releaseAssetArgs), RunE: func(cmd *cobra.Command, args []string) error { tag := args[0] filePath := args[1] @@ -353,7 +358,7 @@ func releaseDownloadCmd() *cobra.Command { Use: "download ", Short: "Download a release asset", Long: "Downloads a release asset by first listing assets to find the matching name, then downloading by ID.", - Args: cobra.ExactArgs(2), + Args: cobra.ExactArgs(releaseAssetArgs), RunE: func(cmd *cobra.Command, args []string) error { tag := args[0] assetName := args[1] diff --git a/internal/cli/release_test.go b/internal/cli/release_test.go index cdd0634..ee682ad 100644 --- a/internal/cli/release_test.go +++ b/internal/cli/release_test.go @@ -6,6 +6,8 @@ import ( "github.com/spf13/cobra" ) +const flagTypeBool = "bool" + func TestReleaseEditDraftFlag(t *testing.T) { cmd := releaseEditCmd() @@ -14,7 +16,7 @@ func TestReleaseEditDraftFlag(t *testing.T) { if f == nil { t.Fatal("expected --draft flag to exist") } - if f.Value.Type() != "bool" { + if f.Value.Type() != flagTypeBool { t.Errorf("expected --draft to be bool, got %s", f.Value.Type()) } @@ -22,7 +24,7 @@ func TestReleaseEditDraftFlag(t *testing.T) { if f == nil { t.Fatal("expected --prerelease flag to exist") } - if f.Value.Type() != "bool" { + if f.Value.Type() != flagTypeBool { t.Errorf("expected --prerelease to be bool, got %s", f.Value.Type()) } } @@ -34,7 +36,7 @@ func TestReleaseCreateDraftFlag(t *testing.T) { if f == nil { t.Fatal("expected --draft flag to exist") } - if f.Value.Type() != "bool" { + if f.Value.Type() != flagTypeBool { t.Errorf("expected --draft to be bool, got %s", f.Value.Type()) } } diff --git a/internal/cli/repo.go b/internal/cli/repo.go index 02d1ac6..9fe55f7 100644 --- a/internal/cli/repo.go +++ b/internal/cli/repo.go @@ -13,6 +13,14 @@ import ( "github.com/spf13/cobra" ) +const ( + defaultRepoLimit = 100 + maxDescLength = 60 + truncatedDescLen = 57 + defaultForkLimit = 30 + defaultSearchLimit = 30 +) + var repoCmd = &cobra.Command{ Use: "repo", Short: "Manage repositories", @@ -161,8 +169,8 @@ func repoListCmd() *cobra.Command { rows := make([][]string, len(repos)) for i, r := range repos { desc := r.Description - if len(desc) > 60 { - desc = desc[:57] + "..." + if len(desc) > maxDescLength { + desc = desc[:truncatedDescLen] + "..." } rows[i] = []string{ r.FullName, @@ -176,7 +184,7 @@ func repoListCmd() *cobra.Command { }, } - cmd.Flags().IntVarP(&flagLimit, "limit", "L", 100, "Maximum number of repos to return") + cmd.Flags().IntVarP(&flagLimit, "limit", "L", defaultRepoLimit, "Maximum number of repos to return") cmd.Flags().BoolVar(&flagNoArchived, "no-archived", false, "Exclude archived repos") cmd.Flags().BoolVar(&flagNoForks, "no-forks", false, "Exclude forked repos") cmd.Flags().BoolVar(&flagArchivedOnly, "archived", false, "Only show archived repos") @@ -237,11 +245,12 @@ func repoCreateCmd() *cobra.Command { return fmt.Errorf("--private, --public, and --internal are mutually exclusive") } - if flagPrivate { + switch { + case flagPrivate: opts.Visibility = forges.VisibilityPrivate - } else if flagInternal { + case flagInternal: opts.Visibility = forges.VisibilityInternal - } else if flagPublic { + case flagPublic: opts.Visibility = forges.VisibilityPublic } @@ -520,8 +529,8 @@ func repoForksCmd() *cobra.Command { rows := make([][]string, len(forks)) for i, r := range forks { desc := r.Description - if len(desc) > 60 { - desc = desc[:57] + "..." + if len(desc) > maxDescLength { + desc = desc[:truncatedDescLen] + "..." } rows[i] = []string{ r.FullName, @@ -534,7 +543,7 @@ func repoForksCmd() *cobra.Command { }, } - cmd.Flags().IntVarP(&flagLimit, "limit", "L", 30, "Maximum number of forks") + cmd.Flags().IntVarP(&flagLimit, "limit", "L", defaultForkLimit, "Maximum number of forks") cmd.Flags().StringVar(&flagSort, "sort", "", "Sort order (newest, oldest, stargazers, watchers)") return cmd } @@ -587,8 +596,8 @@ func repoSearchCmd() *cobra.Command { rows := make([][]string, len(repos)) for i, r := range repos { desc := r.Description - if len(desc) > 60 { - desc = desc[:57] + "..." + if len(desc) > maxDescLength { + desc = desc[:truncatedDescLen] + "..." } rows[i] = []string{ r.FullName, @@ -601,7 +610,7 @@ func repoSearchCmd() *cobra.Command { }, } - cmd.Flags().IntVarP(&flagLimit, "limit", "L", 30, "Maximum number of results") + cmd.Flags().IntVarP(&flagLimit, "limit", "L", defaultSearchLimit, "Maximum number of results") cmd.Flags().StringVar(&flagSort, "sort", "", "Sort field (stars, forks, updated)") cmd.Flags().StringVar(&flagOrder, "order", "", "Sort order (asc, desc)") return cmd diff --git a/internal/cli/review.go b/internal/cli/review.go index b7ea4b2..a6efbda 100644 --- a/internal/cli/review.go +++ b/internal/cli/review.go @@ -11,6 +11,12 @@ import ( "github.com/spf13/cobra" ) +const ( + maxReviewBodyLength = 60 + defaultReviewLimit = 30 + minReviewerArgs = 2 +) + var reviewCmd = &cobra.Command{ Use: "review", Short: "Manage pull request reviews", @@ -74,7 +80,7 @@ func reviewListCmd() *cobra.Command { rows := make([][]string, len(reviews)) for i, r := range reviews { body := r.Body - if len(body) > 60 { + if len(body) > maxReviewBodyLength { body = body[:57] + "..." } rows[i] = []string{ @@ -88,7 +94,7 @@ func reviewListCmd() *cobra.Command { }, } - cmd.Flags().IntVarP(&flagLimit, "limit", "L", 30, "Maximum number of reviews") + cmd.Flags().IntVarP(&flagLimit, "limit", "L", defaultReviewLimit, "Maximum number of reviews") return cmd } @@ -180,7 +186,7 @@ func reviewerRequestCmd() *cobra.Command { return &cobra.Command{ Use: "request ", Short: "Request reviewers on a pull request", - Args: cobra.MinimumNArgs(2), + Args: cobra.MinimumNArgs(minReviewerArgs), RunE: func(cmd *cobra.Command, args []string) error { number, err := strconv.Atoi(args[0]) if err != nil { @@ -206,7 +212,7 @@ func reviewerRemoveCmd() *cobra.Command { return &cobra.Command{ Use: "remove ", Short: "Remove reviewer requests from a pull request", - Args: cobra.MinimumNArgs(2), + Args: cobra.MinimumNArgs(minReviewerArgs), RunE: func(cmd *cobra.Command, args []string) error { number, err := strconv.Atoi(args[0]) if err != nil { diff --git a/internal/cli/search.go b/internal/cli/search.go index 037f07f..e8da0e8 100644 --- a/internal/cli/search.go +++ b/internal/cli/search.go @@ -9,6 +9,8 @@ import ( "github.com/spf13/cobra" ) +const maxSearchDescLength = 50 + var searchCmd = &cobra.Command{ Use: "search", Short: "Search across the forge", @@ -66,7 +68,7 @@ func searchReposCmd() *cobra.Command { rows := make([][]string, len(repos)) for i, r := range repos { desc := r.Description - if len(desc) > 50 { + if len(desc) > maxSearchDescLength { desc = desc[:47] + "..." } rows[i] = []string{ @@ -81,7 +83,7 @@ func searchReposCmd() *cobra.Command { }, } - cmd.Flags().IntVarP(&flagLimit, "limit", "L", 30, "Maximum number of results") + cmd.Flags().IntVarP(&flagLimit, "limit", "L", defaultSearchLimit, "Maximum number of results") cmd.Flags().StringVar(&flagSort, "sort", "", "Sort field: stars, forks, updated") cmd.Flags().StringVar(&flagOrder, "order", "", "Sort order: asc, desc") return cmd diff --git a/internal/config/config.go b/internal/config/config.go index 99e2e58..b76db58 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -10,6 +10,11 @@ import ( "sync" ) +const ( + dirPermissions = 0700 + filePermissions = 0600 +) + type Config struct { Default DefaultSection Domains map[string]DomainSection @@ -199,7 +204,7 @@ func SetDomain(domain, token, forgeType string) error { return fmt.Errorf("cannot determine config path") } - if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + if err := os.MkdirAll(filepath.Dir(path), dirPermissions); err != nil { return err } @@ -240,7 +245,7 @@ func writeINI(path string, sections map[string]map[string]string) error { writeSection(&b, name, kv) } - return os.WriteFile(path, []byte(b.String()), 0600) + return os.WriteFile(path, []byte(b.String()), filePermissions) } func writeSection(b *strings.Builder, name string, kv map[string]string) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e9573b1..b517bc3 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -8,6 +8,8 @@ import ( "testing" ) +const sectionDefault = "default" + func TestParseINI(t *testing.T) { input := ` # This is a comment @@ -30,11 +32,11 @@ token = abc123 t.Fatalf("unexpected error: %v", err) } - if sections["default"]["output"] != "json" { - t.Errorf("expected output=json, got %q", sections["default"]["output"]) + if sections[sectionDefault]["output"] != "json" { + t.Errorf("expected output=json, got %q", sections[sectionDefault]["output"]) } - if sections["default"]["forge-type"] != "gitlab" { - t.Errorf("expected forge-type=gitlab, got %q", sections["default"]["forge-type"]) + if sections[sectionDefault]["forge-type"] != "gitlab" { + t.Errorf("expected forge-type=gitlab, got %q", sections[sectionDefault]["forge-type"]) } if sections["github.com"]["token"] != "ghp_abc123" { t.Errorf("expected github.com token=ghp_abc123, got %q", sections["github.com"]["token"]) @@ -64,7 +66,7 @@ func TestParseINIKeyBeforeSection(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if sections["default"]["output"] != "table" { + if sections[sectionDefault]["output"] != "table" { t.Errorf("expected key before section to land in default, got %v", sections) } } @@ -166,7 +168,7 @@ type = github // Simulate loading as project config (allowTokens=false) for name, kv := range sections { - if name == "default" { + if name == sectionDefault { continue } ds := cfg.Domains[name] diff --git a/internal/output/output.go b/internal/output/output.go index 8dd3c01..1ee8739 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -15,6 +15,8 @@ const ( Table Format = "table" JSON Format = "json" Plain Format = "plain" + + tabPadding = 2 ) // ParseFormat converts a string flag value to a Format. @@ -45,7 +47,7 @@ func (p *Printer) PrintJSON(v any) error { // PrintTable writes rows as a tab-aligned table. headers are the column names; // rows is a slice of slices where each inner slice is one row. func (p *Printer) PrintTable(headers []string, rows [][]string) { - w := tabwriter.NewWriter(p.Out, 0, 0, 2, ' ', 0) + w := tabwriter.NewWriter(p.Out, 0, 0, tabPadding, ' ', 0) _, _ = fmt.Fprintln(w, strings.Join(headers, "\t")) for _, row := range rows { _, _ = fmt.Fprintln(w, strings.Join(row, "\t")) diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index 1d3955e..8d55f5e 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -3,7 +3,6 @@ package resolve import ( "context" "fmt" - "net/http" "os" "os/exec" "strings" @@ -16,16 +15,12 @@ import ( "github.com/git-pkgs/forge/internal/config" ) +const ownerRepoParts = 2 + var builders = forges.ForgeBuilders{ - GitHub: func(baseURL, token string, hc *http.Client) forges.Forge { - return ghforge.NewWithBase(baseURL, token, hc) - }, - GitLab: func(baseURL, token string, hc *http.Client) forges.Forge { - return glforge.New(baseURL, token, hc) - }, - Gitea: func(baseURL, token string, hc *http.Client) forges.Forge { - return gitea.New(baseURL, token, hc) - }, + GitHub: ghforge.NewWithBase, + GitLab: glforge.New, + Gitea: gitea.New, } // Repo figures out the forge, owner, and repo name from flags or the current @@ -39,8 +34,8 @@ func Repo(flagRepo, flagForgeType string) (forge forges.Forge, owner, repo, doma } func repoFromFlag(flagRepo, flagForgeType string) (forges.Forge, string, string, string, error) { - parts := strings.SplitN(flagRepo, "/", 2) - if len(parts) != 2 { + parts := strings.SplitN(flagRepo, "/", ownerRepoParts) + if len(parts) != ownerRepoParts { return nil, "", "", "", fmt.Errorf("invalid repo format %q, expected OWNER/REPO", flagRepo) } owner, repo := parts[0], parts[1] @@ -54,7 +49,7 @@ func repoFromFlag(flagRepo, flagForgeType string) (forges.Forge, string, string, return f, owner, repo, domain, nil } -func repoFromGitRemote(flagForgeType string) (forges.Forge, string, string, string, error) { +func repoFromGitRemote(_ string) (forges.Forge, string, string, string, error) { url, err := gitRemoteURL("origin") if err != nil { return nil, "", "", "", fmt.Errorf("not in a git repo and -R not set: %w", err)