From 00597ec59d9196687e7a6a822fe0bbb7aedfba54 Mon Sep 17 00:00:00 2001 From: Eoin McAfee Date: Mon, 27 Sep 2021 10:33:33 +0100 Subject: [PATCH] techqa feedback --- scm/driver/bitbucket/milestone.go | 2 +- scm/driver/gitea/milestone.go | 32 ++++++++++++++--------------- scm/driver/gitea/milestone_test.go | 4 ++-- scm/driver/gitea/release.go | 9 +++++++- scm/driver/gitea/repo.go | 6 +++--- scm/driver/gitea/util.go | 24 +++++++++------------- scm/driver/github/milestone.go | 8 ++++---- scm/driver/github/milestone_test.go | 4 ++-- scm/driver/gitlab/milestone.go | 20 +++++++++--------- scm/driver/gitlab/milestone_test.go | 4 ++-- scm/driver/gitlab/release.go | 6 ++---- scm/milestone.go | 2 +- scm/pr.go | 2 +- 13 files changed, 62 insertions(+), 61 deletions(-) diff --git a/scm/driver/bitbucket/milestone.go b/scm/driver/bitbucket/milestone.go index 7e18430ae..7c2624dc6 100644 --- a/scm/driver/bitbucket/milestone.go +++ b/scm/driver/bitbucket/milestone.go @@ -14,7 +14,7 @@ func (s *milestoneService) Find(ctx context.Context, repo string, id int) (*scm. return nil, nil, scm.ErrNotSupported } -func (s *milestoneService) List(ctx context.Context, repo string, opts scm.ListOptions) ([]*scm.Milestone, *scm.Response, error) { +func (s *milestoneService) List(ctx context.Context, repo string, opts scm.MilestoneListOptions) ([]*scm.Milestone, *scm.Response, error) { return nil, nil, scm.ErrNotSupported } diff --git a/scm/driver/gitea/milestone.go b/scm/driver/gitea/milestone.go index bec60a56e..545a54775 100644 --- a/scm/driver/gitea/milestone.go +++ b/scm/driver/gitea/milestone.go @@ -67,7 +67,7 @@ func (s *milestoneService) Update(ctx context.Context, repo string, id int, inpu if input.Description != "" { in.Description = input.Description } - if input.DueDate != nil { + if !input.DueDate.IsZero() { in.Deadline = input.DueDate } out := new(milestone) @@ -88,23 +88,23 @@ const ( ) type milestone struct { - ID int64 `json:"id"` - Title string `json:"title"` - Description string `json:"description"` - State StateType `json:"state"` - OpenIssues int `json:"open_issues"` - ClosedIssues int `json:"closed_issues"` - Created time.Time `json:"created_at"` - Updated *time.Time `json:"updated_at"` - Closed *time.Time `json:"closed_at"` - Deadline *time.Time `json:"due_on"` + ID int64 `json:"id"` + Title string `json:"title"` + Description string `json:"description"` + State StateType `json:"state"` + OpenIssues int `json:"open_issues"` + ClosedIssues int `json:"closed_issues"` + Created time.Time `json:"created_at"` + Updated time.Time `json:"updated_at"` + Closed time.Time `json:"closed_at"` + Deadline time.Time `json:"due_on"` } type milestoneInput struct { - Title string `json:"title"` - Description string `json:"description"` - State StateType `json:"state"` - Deadline *time.Time `json:"due_on"` + Title string `json:"title"` + Description string `json:"description"` + State StateType `json:"state"` + Deadline time.Time `json:"due_on"` } func convertMilestoneList(src []*milestone) []*scm.Milestone { @@ -116,7 +116,7 @@ func convertMilestoneList(src []*milestone) []*scm.Milestone { } func convertMilestone(src *milestone) *scm.Milestone { - if src == nil || src.Deadline == nil { + if src == nil || src.Deadline.IsZero() { return nil } return &scm.Milestone{ diff --git a/scm/driver/gitea/milestone_test.go b/scm/driver/gitea/milestone_test.go index f825ae161..93d9f185e 100644 --- a/scm/driver/gitea/milestone_test.go +++ b/scm/driver/gitea/milestone_test.go @@ -94,7 +94,7 @@ func TestMilestoneCreate(t *testing.T) { Title: "v1.0", Description: "Tracking milestone for version 1.0", State: "open", - DueDate: &dueDate, + DueDate: dueDate, } got, _, err := client.Milestones.Create(context.Background(), "jcitizen/my-repo", input) if err != nil { @@ -132,7 +132,7 @@ func TestMilestoneUpdate(t *testing.T) { Title: "v1.0", Description: "Tracking milestone for version 1.0", State: "open", - DueDate: &dueDate, + DueDate: dueDate, } got, _, err := client.Milestones.Update(context.Background(), "jcitizen/my-repo", 1, input) if err != nil { diff --git a/scm/driver/gitea/release.go b/scm/driver/gitea/release.go index 7658fb7c3..a85b57cb0 100644 --- a/scm/driver/gitea/release.go +++ b/scm/driver/gitea/release.go @@ -30,7 +30,7 @@ func (s *releaseService) FindByTag(ctx context.Context, repo string, tag string) func (s *releaseService) List(ctx context.Context, repo string, opts scm.ReleaseListOptions) ([]*scm.Release, *scm.Response, error) { namespace, name := scm.Split(repo) - path := fmt.Sprintf("api/v1/repos/%s/%s/releases?%s", namespace, name, encodeReleaseListOptions(opts)) + path := fmt.Sprintf("api/v1/repos/%s/%s/releases?%s", namespace, name, encodeReleaseListOptions(releaseListOptionsToGiteaListOptions(opts))) out := []*release{} res, err := s.client.do(ctx, "GET", path, nil, &out) return convertReleaseList(out), res, err @@ -148,3 +148,10 @@ func convertReleaseList(src []*release) []*scm.Release { } return dst } + +func releaseListOptionsToGiteaListOptions(in scm.ReleaseListOptions) ListOptions { + return ListOptions{ + Page: in.Page, + PageSize: in.Size, + } +} \ No newline at end of file diff --git a/scm/driver/gitea/repo.go b/scm/driver/gitea/repo.go index ef2b441bd..154c7f322 100644 --- a/scm/driver/gitea/repo.go +++ b/scm/driver/gitea/repo.go @@ -135,9 +135,9 @@ func (s *repositoryService) DeleteHook(ctx context.Context, repo string, id stri type ( // gitea repository resource. repository struct { - ID int `json:"id"` - Owner user `json:"owner"` - Name string `json:"name"` + ID int `json:"id"` + Owner user `json:"owner"` + Name string `json:"name"` FullName string `json:"full_name"` Private bool `json:"private"` Fork bool `json:"fork"` diff --git a/scm/driver/gitea/util.go b/scm/driver/gitea/util.go index e88ed4c66..3404b1b0c 100644 --- a/scm/driver/gitea/util.go +++ b/scm/driver/gitea/util.go @@ -90,18 +90,14 @@ func encodeMilestoneListOptions(opts scm.MilestoneListOptions) string { return params.Encode() } -func encodeReleaseListOptions(opts scm.ReleaseListOptions) string { - params := url.Values{} - if opts.Page != 0 { - params.Set("page", strconv.Itoa(opts.Page)) - } - if opts.Size != 0 { - params.Set("per_page", strconv.Itoa(opts.Size)) - } - if opts.Open && opts.Closed { - params.Set("state", "all") - } else if opts.Closed { - params.Set("state", "closed") - } - return params.Encode() +type ListOptions struct { + Page int + PageSize int +} + +func encodeReleaseListOptions(o ListOptions) string { + query := make(url.Values) + query.Add("page", fmt.Sprintf("%d", o.Page)) + query.Add("limit", fmt.Sprintf("%d", o.PageSize)) + return query.Encode() } \ No newline at end of file diff --git a/scm/driver/github/milestone.go b/scm/driver/github/milestone.go index beb8847e7..b01c0e1c5 100644 --- a/scm/driver/github/milestone.go +++ b/scm/driver/github/milestone.go @@ -58,7 +58,7 @@ func (s *milestoneService) Create(ctx context.Context, repo string, input *scm.M Title: input.Title, State: input.State, Description: input.Description, - DueOn: *input.DueDate, + DueOn: input.DueDate, } out := new(milestone) res, err := s.client.do(ctx, "POST", path, in, out) @@ -82,8 +82,8 @@ func (s *milestoneService) Update(ctx context.Context, repo string, id int, inpu if input.Description != "" { in.Description = input.Description } - if input.DueDate != nil { - in.DueOn = *input.DueDate + if !input.DueDate.IsZero() { + in.DueOn = input.DueDate } out := new(milestone) res, err := s.client.do(ctx, "PATCH", path, in, out) @@ -106,6 +106,6 @@ func convertMilestone(from *milestone) *scm.Milestone { Description: from.Description, Link: from.HTMLURL, State: from.State, - DueDate: &from.DueOn, + DueDate: from.DueOn, } } diff --git a/scm/driver/github/milestone_test.go b/scm/driver/github/milestone_test.go index e3546b36d..0d9c72d7a 100644 --- a/scm/driver/github/milestone_test.go +++ b/scm/driver/github/milestone_test.go @@ -98,7 +98,7 @@ func TestMilestoneCreate(t *testing.T) { Title: "v1.0", Description: "Tracking milestone for version 1.0", State: "open", - DueDate: &dueDate, + DueDate: dueDate, } got, res, err := client.Milestones.Create(context.Background(), "octocat/hello-world", input) @@ -140,7 +140,7 @@ func TestMilestoneUpdate(t *testing.T) { Title: "v1.0", Description: "Tracking milestone for version 1.0", State: "open", - DueDate: &dueDate, + DueDate: dueDate, } got, res, err := client.Milestones.Update(context.Background(), "octocat/hello-world", 1, input) diff --git a/scm/driver/gitlab/milestone.go b/scm/driver/gitlab/milestone.go index 7cdf8e56a..0995a5ebf 100644 --- a/scm/driver/gitlab/milestone.go +++ b/scm/driver/gitlab/milestone.go @@ -77,10 +77,10 @@ type milestone struct { } type milestoneInput struct { - Title *string `json:"title"` - StateEvent *string `json:"state_event,omitempty"` - Description *string `json:"description"` - DueDate *isoTime `json:"due_date"` + Title *string `json:"title"` + StateEvent *string `json:"state_event,omitempty"` + Description *string `json:"description"` + DueDate isoTime `json:"due_date"` } func (s *milestoneService) Find(ctx context.Context, repo string, id int) (*scm.Milestone, *scm.Response, error) { @@ -99,11 +99,11 @@ func (s *milestoneService) List(ctx context.Context, repo string, opts scm.Miles func (s *milestoneService) Create(ctx context.Context, repo string, input *scm.MilestoneInput) (*scm.Milestone, *scm.Response, error) { path := fmt.Sprintf("api/v4/projects/%s/milestones", encode(repo)) - dueDateIso := isoTime(*input.DueDate) + dueDateIso := isoTime(input.DueDate) in := &milestoneInput{ Title: &input.Title, Description: &input.Description, - DueDate: &dueDateIso, + DueDate: dueDateIso, } out := new(milestone) res, err := s.client.do(ctx, "POST", path, in, out) @@ -133,9 +133,9 @@ func (s *milestoneService) Update(ctx context.Context, repo string, id int, inpu if input.Description != "" { in.Description = &input.Description } - if input.DueDate != nil { - dueDateIso := isoTime(*input.DueDate) - in.DueDate = &dueDateIso + if !input.DueDate.IsZero() { + dueDateIso := isoTime(input.DueDate) + in.DueDate = dueDateIso } out := new(milestone) res, err := s.client.do(ctx, "PATCH", path, in, out) @@ -161,6 +161,6 @@ func convertMilestone(from *milestone) *scm.Milestone { Title: from.Title, Description: from.Description, State: from.State, - DueDate: &dueDate, + DueDate: dueDate, } } diff --git a/scm/driver/gitlab/milestone_test.go b/scm/driver/gitlab/milestone_test.go index f8e02794c..58d578638 100644 --- a/scm/driver/gitlab/milestone_test.go +++ b/scm/driver/gitlab/milestone_test.go @@ -99,7 +99,7 @@ func TestMilestoneCreate(t *testing.T) { Title: "v1.0", Description: "Tracking milestone for version 1.0", State: "open", - DueDate: &dueDate, + DueDate: dueDate, } got, res, err := client.Milestones.Create(context.Background(), "diaspora/diaspora", input) if err != nil { @@ -142,7 +142,7 @@ func TestMilestoneUpdate(t *testing.T) { Title: "v1.0", Description: "Tracking milestone for version 1.0", State: "close", - DueDate: &dueDate, + DueDate: dueDate, } got, res, err := client.Milestones.Update(context.Background(), "diaspora/diaspora", 1, input) if err != nil { diff --git a/scm/driver/gitlab/release.go b/scm/driver/gitlab/release.go index 625404870..874c728ee 100644 --- a/scm/driver/gitlab/release.go +++ b/scm/driver/gitlab/release.go @@ -27,8 +27,7 @@ type releaseInput struct { } func (s *releaseService) Find(ctx context.Context, repo string, id int) (*scm.Release, *scm.Response, error) { - // this could be implemented by List and filter but would be to expensive - panic("gitlab only allows to find a release by tag") + return nil, nil, scm.ErrNotSupported } func (s *releaseService) FindByTag(ctx context.Context, repo string, tag string) (*scm.Release, *scm.Response, error) { @@ -58,8 +57,7 @@ func (s *releaseService) Create(ctx context.Context, repo string, input *scm.Rel } func (s *releaseService) Delete(ctx context.Context, repo string, id int) (*scm.Response, error) { - // this could be implemented by List and filter but would be to expensive - panic("gitlab only allows to delete a release by tag") + return nil, scm.ErrNotSupported } func (s *releaseService) DeleteByTag(ctx context.Context, repo string, tag string) (*scm.Response, error) { diff --git a/scm/milestone.go b/scm/milestone.go index 54ed9683b..a251c9dba 100644 --- a/scm/milestone.go +++ b/scm/milestone.go @@ -11,7 +11,7 @@ type ( Title string Description string State string - DueDate *time.Time + DueDate time.Time } // MilestoneListOptions provides options for querying a list of repository milestones. diff --git a/scm/pr.go b/scm/pr.go index 49d4e3543..f08147bae 100644 --- a/scm/pr.go +++ b/scm/pr.go @@ -72,7 +72,7 @@ type ( Description string Link string State string - DueDate *time.Time + DueDate time.Time } // PullRequestService provides access to pull request resources.