Few minor adjustments to finish the PR

xanzy · Sep 2, 2023 · c9b2e77 · c9b2e77
1 parent 4a23587
commit c9b2e77
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 83 deletions.
diff --git a/merge_trains.go b/merge_trains.go
@@ -20,8 +20,8 @@ type MergeTrainsService struct {
 type MergeTrain struct {
 	ID           int                     `json:"id"`
 	MergeRequest *MergeTrainMergeRequest `json:"merge_request"`
-	User         *MergeTrainUser         `json:"user"`
-	Pipeline     *MergeTrainPipeline     `json:"pipeline"`
+	User         *BasicUser              `json:"user"`
+	Pipeline     *Pipeline               `json:"pipeline"`
 	CreatedAt    *time.Time              `json:"created_at"`
 	UpdatedAt    *time.Time              `json:"updated_at"`
 	TargetBranch string                  `json:"target_branch"`
@@ -30,7 +30,9 @@ type MergeTrain struct {
 	Duration     int                     `json:"duration"`
 }
 
-// MergeTrainMergeRequest represents a Gitlab merge request inside merge train
+// MergeTrainMergeRequest represents a Gitlab merge request inside merge train.
+//
+// GitLab API docs: https://docs.gitlab.com/ee/api/merge_trains.html
 type MergeTrainMergeRequest struct {
 	ID          int        `json:"id"`
 	IID         int        `json:"iid"`
@@ -43,30 +45,6 @@ type MergeTrainMergeRequest struct {
 	WebURL      string     `json:"web_url"`
 }
 
-// MergeTrainUser represents a Gitlab user inside merge train
-type MergeTrainUser struct {
-	ID        int    `json:"id"`
-	Name      string `json:"name"`
-	Username  string `json:"username"`
-	State     string `json:"state"`
-	AvatarURL string `json:"avatar_url"`
-	WebURL    string `json:"web_url"`
-}
-
-// MergeTrainPipeline represents a Gitlab pipeline inside merge train
-type MergeTrainPipeline struct {
-	ID        int        `json:"id"`
-	IID       int        `json:"iid"`
-	ProjectID int        `json:"project_id"`
-	SHA       string     `json:"sha"`
-	Ref       string     `json:"ref"`
-	Status    string     `json:"status"`
-	Source    string     `json:"source"`
-	CreatedAt *time.Time `json:"created_at"`
-	UpdatedAt *time.Time `json:"updated_at"`
-	WebURL    string     `json:"web_url"`
-}
-
 // ListMergeTrainsOptions represents the available ListMergeTrain() options.
 //
 // Gitab API docs:
@@ -77,9 +55,7 @@ type ListMergeTrainsOptions struct {
 	Sort  *string `url:"sort,omitempty" json:"sort,omitempty"`
 }
 
-// ListProjectMergeTrains get a list of merge trains in a project
-//
-//	The scope of trains to show: active (to be merged) and complete (have been merged)
+// ListProjectMergeTrains get a list of merge trains in a project.
 //
 // GitLab API docs:
 // https://docs.gitlab.com/ee/api/merge_trains.html#list-merge-trains-for-a-project
@@ -88,25 +64,24 @@ func (s *MergeTrainsService) ListProjectMergeTrains(pid interface{}, opt *ListMe
 	if err != nil {
 		return nil, nil, err
 	}
-
 	u := fmt.Sprintf("projects/%s/merge_trains", PathEscape(project))
 
 	req, err := s.client.NewRequest(http.MethodGet, u, opt, options)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	var mergeTrains []*MergeTrain
-	resp, err := s.client.Do(req, &mergeTrains)
+	var mts []*MergeTrain
+	resp, err := s.client.Do(req, &mts)
 	if err != nil {
 		return nil, resp, err
 	}
 
-	return mergeTrains, resp, nil
+	return mts, resp, nil
 }
 
-// ListMergeRequestInMergeTrain gets a list of merge requests added to a merge train
-// for the requested target branch
+// ListMergeRequestInMergeTrain gets a list of merge requests added to a merge
+// train for the requested target branch.
 //
 // GitLab API docs:
 // https://docs.gitlab.com/ee/api/merge_trains.html#list-merge-requests-in-a-merge-train
@@ -115,79 +90,81 @@ func (s *MergeTrainsService) ListMergeRequestInMergeTrain(pid interface{}, targe
 	if err != nil {
 		return nil, nil, err
 	}
-
 	u := fmt.Sprintf("projects/%s/merge_trains/%s", PathEscape(project), targetBranch)
 
 	req, err := s.client.NewRequest(http.MethodGet, u, opts, options)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	var mergeTrains []*MergeTrain
-	resp, err := s.client.Do(req, &mergeTrains)
+	var mts []*MergeTrain
+	resp, err := s.client.Do(req, &mts)
 	if err != nil {
 		return nil, resp, err
 	}
 
-	return mergeTrains, resp, nil
+	return mts, resp, nil
 }
 
-// GetMergeRequestOnAMergeTrain Get merge train information for the requested merge request.
+// GetMergeRequestOnAMergeTrain Get merge train information for the requested
+// merge request.
 //
-// GitLab API docs: https://docs.gitlab.com/ee/api/merge_trains.html#get-the-status-of-a-merge-request-on-a-merge-train
+// GitLab API docs:
+// https://docs.gitlab.com/ee/api/merge_trains.html#get-the-status-of-a-merge-request-on-a-merge-train
 func (s *MergeTrainsService) GetMergeRequestOnAMergeTrain(pid interface{}, mergeRequest int, options ...RequestOptionFunc) (*MergeTrain, *Response, error) {
 	project, err := parseID(pid)
 	if err != nil {
 		return nil, nil, err
 	}
-
 	u := fmt.Sprintf("projects/%s/merge_trains/merge_requests/%d", PathEscape(project), mergeRequest)
 
 	req, err := s.client.NewRequest(http.MethodGet, u, nil, options)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	var mergeTrain *MergeTrain
-	resp, err := s.client.Do(req, &mergeTrain)
+	mt := new(MergeTrain)
+	resp, err := s.client.Do(req, mt)
 	if err != nil {
 		return nil, resp, err
 	}
 
-	return mergeTrain, resp, nil
+	return mt, resp, nil
 }
 
-// AddMergeRequestToMergeTrainOptions represents the available AddMergeRequestToMergeTrain() options.
+// AddMergeRequestToMergeTrainOptions represents the available
+// AddMergeRequestToMergeTrain() options.
 //
 // GitLab API docs:
 // https://docs.gitlab.com/ee/api/merge_trains.html#add-a-merge-request-to-a-merge-train
 type AddMergeRequestToMergeTrainOptions struct {
-	WhenPipelineSucceeds *bool   `json:"when_pipeline_succeeds,omitempty"`
-	SHA                  *string `json:"sha,omitempty"`
-	Squash               *string `json:"squash,omitempty"`
+	WhenPipelineSucceeds *bool   `url:"when_pipeline_succeeds,omitempty" json:"when_pipeline_succeeds,omitempty"`
+	SHA                  *string `url:"sha,omitempty" json:"sha,omitempty"`
+	Squash               *string `url:"squash,omitempty" json:"squash,omitempty"`
 }
 
-// AddMergeRequestToMergeTrain Add a merge request to the merge train targeting the merge request’s target branch
+// AddMergeRequestToMergeTrain Add a merge request to the merge train targeting
+// the merge request’s target branch.
 //
-// GitLab API docs: https://docs.gitlab.com/ee/api/merge_trains.html#add-a-merge-request-to-a-merge-train
+// GitLab API docs:
+// https://docs.gitlab.com/ee/api/merge_trains.html#add-a-merge-request-to-a-merge-train
 func (s *MergeTrainsService) AddMergeRequestToMergeTrain(pid interface{}, mergeRequest int, opts *AddMergeRequestToMergeTrainOptions, options ...RequestOptionFunc) ([]*MergeTrain, *Response, error) {
 	project, err := parseID(pid)
 	if err != nil {
 		return nil, nil, err
 	}
-
 	u := fmt.Sprintf("projects/%s/merge_trains/merge_requests/%d", PathEscape(project), mergeRequest)
 
 	req, err := s.client.NewRequest(http.MethodPost, u, opts, options)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	var mergeTrains []*MergeTrain
-	resp, err := s.client.Do(req, &mergeTrains)
+	var mts []*MergeTrain
+	resp, err := s.client.Do(req, &mts)
 	if err != nil {
 		return nil, resp, err
 	}
 
-	return mergeTrains, resp, nil
+	return mts, resp, nil
 }
diff --git a/merge_trains_test.go b/merge_trains_test.go
@@ -46,15 +46,15 @@ func TestListProjectMergeTrains(t *testing.T) {
 				UpdatedAt:   &mergeRequestUpdatedAt,
 				WebURL:      "http://local.gitlab.test:8181/root/merge-train-race-condition/-/merge_requests/59",
 			},
-			User: &MergeTrainUser{
+			User: &BasicUser{
 				ID:        1,
 				Name:      "Administrator",
 				Username:  "root",
 				State:     "active",
 				AvatarURL: "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon",
 				WebURL:    "http://local.gitlab.test:8181/root",
 			},
-			Pipeline: &MergeTrainPipeline{
+			Pipeline: &Pipeline{
 				ID:        246,
 				SHA:       "bcc17a8ffd51be1afe45605e714085df28b80b13",
 				Ref:       "refs/merge-requests/59/train",
@@ -115,15 +115,15 @@ func TestListMergeRequestInMergeTrain(t *testing.T) {
 				UpdatedAt:   &mergeRequestUpdatedAt,
 				WebURL:      "http://localhost/namespace18/project21/-/merge_requests/1",
 			},
-			User: &MergeTrainUser{
+			User: &BasicUser{
 				ID:        933,
 				Username:  "user12",
 				Name:      "Sidney Jones31",
 				State:     "active",
 				AvatarURL: "https://www.gravatar.com/avatar/6c8365de387cb3db10ecc7b1880203c4?s=80\u0026d=identicon",
 				WebURL:    "http://localhost/user12",
 			},
-			Pipeline: &MergeTrainPipeline{
+			Pipeline: &Pipeline{
 				ID:        273,
 				IID:       1,
 				ProjectID: 598,
@@ -184,15 +184,15 @@ func TestGetMergeRequestOnAMergeTrain(t *testing.T) {
 			UpdatedAt:   &mergeRequestUpdatedAt,
 			WebURL:      "http://localhost/namespace18/project21/-/merge_requests/1",
 		},
-		User: &MergeTrainUser{
+		User: &BasicUser{
 			ID:        933,
 			Username:  "user12",
 			Name:      "Sidney Jones31",
 			State:     "active",
 			AvatarURL: "https://www.gravatar.com/avatar/6c8365de387cb3db10ecc7b1880203c4?s=80\u0026d=identicon",
 			WebURL:    "http://localhost/user12",
 		},
-		Pipeline: &MergeTrainPipeline{
+		Pipeline: &Pipeline{
 			ID:        273,
 			IID:       1,
 			ProjectID: 598,
@@ -255,15 +255,15 @@ func TestAddMergeRequestToMergeTrain(t *testing.T) {
 				UpdatedAt:   &mergeRequestUpdatedAt,
 				WebURL:      "http://localhost/namespace18/project21/-/merge_requests/1",
 			},
-			User: &MergeTrainUser{
+			User: &BasicUser{
 				ID:        933,
 				Username:  "user12",
 				Name:      "Sidney Jones31",
 				State:     "active",
 				AvatarURL: "https://www.gravatar.com/avatar/6c8365de387cb3db10ecc7b1880203c4?s=80\u0026d=identicon",
 				WebURL:    "http://localhost/user12",
 			},
-			Pipeline: &MergeTrainPipeline{
+			Pipeline: &Pipeline{
 				ID:        273,
 				IID:       1,
 				ProjectID: 598,

diff --git a/testdata/add_merge_request_in_merge_train.json b/testdata/add_merge_request_in_merge_train.json
@@ -33,10 +33,10 @@
       "web_url": "http://localhost/namespace19/project22/-/pipelines/273"
     },
     "created_at": "2022-10-31T19:06:06.237Z",
-    "updated_at":"2022-10-31T19:06:06.237Z",
-    "target_branch":"main",
-    "status":"idle",
-    "merged_at":null,
-    "duration":null
+    "updated_at": "2022-10-31T19:06:06.237Z",
+    "target_branch": "main",
+    "status": "idle",
+    "merged_at": null,
+    "duration": null
   }
-]
+]
diff --git a/testdata/get_merge_request_in_merge_train.json b/testdata/get_merge_request_in_merge_train.json
@@ -32,9 +32,9 @@
     "web_url": "http://localhost/namespace19/project22/-/pipelines/273"
   },
   "created_at": "2022-10-31T19:06:06.237Z",
-  "updated_at":"2022-10-31T19:06:06.237Z",
-  "target_branch":"main",
-  "status":"idle",
-  "merged_at":null,
-  "duration":null
-}
+  "updated_at": "2022-10-31T19:06:06.237Z",
+  "target_branch": "main",
+  "status": "idle",
+  "merged_at": null,
+  "duration": null
+}
diff --git a/testdata/list_merge_requests_in_merge_train.json b/testdata/list_merge_requests_in_merge_train.json
@@ -33,10 +33,10 @@
       "web_url": "http://localhost/namespace19/project22/-/pipelines/273"
     },
     "created_at": "2022-10-31T19:06:06.237Z",
-    "updated_at":"2022-10-31T19:06:06.237Z",
-    "target_branch":"main",
-    "status":"idle",
-    "merged_at":null,
-    "duration":null
+    "updated_at": "2022-10-31T19:06:06.237Z",
+    "target_branch": "main",
+    "status": "idle",
+    "merged_at": null,
+    "duration": null
   }
-]
+]
diff --git a/testdata/list_merge_trains_in_project.json b/testdata/list_merge_trains_in_project.json
@@ -36,4 +36,4 @@
     "merged_at": "2020-02-06T08:40:57.719Z",
     "duration": 70
   }
-]
+]