Fix linting errors and refactor build variables code (#113)

variables.go → build_variables.go

@@ -5,32 +5,35 @@ import (
 	"net/url"
 )
 
-// VariablesService handles communication with the project variables related methods
+// BuildVariablesService handles communication with the project variables related methods
 // of the Gitlab API
 //
 // Gitlab API Docs : https://docs.gitlab.com/ce/api/build_variables.html
-type VariablesService struct {
+type BuildVariablesService struct {
 	client *Client
 }
 
-//Variable represents a variable available for each build of the given project
+// BuildVariable represents a variable available for each build of the given project
 //
 // Gitlab API Docs : https://docs.gitlab.com/ce/api/build_variables.html
-type Variable struct {
+type BuildVariable struct {
 	Key   string `json:"key"`
 	Value string `json:"value"`
 }
 
-// ListVariables gets the a list of project variables in a project
+func (v BuildVariable) String() string {
+	return Stringify(v)
+}
+
+// ListBuildVariables gets the a list of project variables in a project
 //
 // Gitlab API Docs:
 // https://docs.gitlab.com/ce/api/build_variables.html#list-project-variables
-func (s *VariablesService) ListVariables(pid interface{}) ([]*Variable, *Response, error) {
+func (s *BuildVariablesService) ListBuildVariables(pid interface{}) ([]*BuildVariable, *Response, error) {
 	project, err := parseID(pid)
 	if err != nil {
 		return nil, nil, err
 	}
-
 	u := fmt.Sprintf("projects/%s/variables", url.QueryEscape(project))
 
 	req, err := s.client.NewRequest("GET", u, nil)
@@ -38,105 +41,101 @@ func (s *VariablesService) ListVariables(pid interface{}) ([]*Variable, *Respons
 		return nil, nil, err
 	}
 
-	var variables []*Variable
-	resp, err := s.client.Do(req, &variables)
+	var v []*BuildVariable
+	resp, err := s.client.Do(req, &v)
 	if err != nil {
 		return nil, resp, err
 	}
 
-	return variables, resp, err
+	return v, resp, err
 }
 
-// GetSingleVariable gets a single project variable of a project
+// GetBuildVariable gets a single project variable of a project
 //
 // Gitlab API Docs:
 // https://docs.gitlab.com/ce/api/build_variables.html#show-variable-details
-func (s *VariablesService) GetSingleVariable(pid interface{}, variableKey string) (*Variable, *Response, error) {
+func (s *BuildVariablesService) GetBuildVariable(pid interface{}, key string) (*BuildVariable, *Response, error) {
 	project, err := parseID(pid)
 	if err != nil {
 		return nil, nil, err
 	}
-
-	u := fmt.Sprintf("projects/%s/variables/%s", url.QueryEscape(project), variableKey)
+	u := fmt.Sprintf("projects/%s/variables/%s", url.QueryEscape(project), key)
 
 	req, err := s.client.NewRequest("GET", u, nil)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	variable := new(Variable)
-	resp, err := s.client.Do(req, variable)
+	v := new(BuildVariable)
+	resp, err := s.client.Do(req, v)
 	if err != nil {
 		return nil, resp, err
 	}
 
-	return variable, resp, err
+	return v, resp, err
 }
 
-// CreateVariable creates a variable for a given project
+// CreateBuildVariable creates a variable for a given project
 //
 // Gitlab API Docs:
 // https://docs.gitlab.com/ce/api/build_variables.html#create-variable
-func (s *VariablesService) CreateVariable(pid interface{}, variable Variable) (*Variable, *Response, error) {
+func (s *BuildVariablesService) CreateBuildVariable(pid interface{}, key, value string) (*BuildVariable, *Response, error) {
 	project, err := parseID(pid)
 	if err != nil {
 		return nil, nil, err
 	}
-
 	u := fmt.Sprintf("projects/%s/variables", url.QueryEscape(project))
 
-	req, err := s.client.NewRequest("POST", u, variable)
+	req, err := s.client.NewRequest("POST", u, BuildVariable{key, value})
 	if err != nil {
 		return nil, nil, err
 	}
 
-	createdVariable := new(Variable)
-	resp, err := s.client.Do(req, createdVariable)
+	v := new(BuildVariable)
+	resp, err := s.client.Do(req, v)
 	if err != nil {
 		return nil, resp, err
 	}
 
-	return createdVariable, resp, err
+	return v, resp, err
 }
 
-// UpdateVariable updates an existing project variable
+// UpdateBuildVariable updates an existing project variable
 // The variable key must exist
 //
 // Gitlab API Docs:
 // https://docs.gitlab.com/ce/api/build_variables.html#update-variable
-func (s *VariablesService) UpdateVariable(pid interface{}, variableKey string, variable Variable) (*Variable, *Response, error) {
+func (s *BuildVariablesService) UpdateBuildVariable(pid interface{}, key, value string) (*BuildVariable, *Response, error) {
 	project, err := parseID(pid)
 	if err != nil {
 		return nil, nil, err
 	}
+	u := fmt.Sprintf("projects/%s/variables/%s", url.QueryEscape(project), key)
 
-	u := fmt.Sprintf("projects/%s/variables/%s", url.QueryEscape(project), variableKey)
-
-	req, err := s.client.NewRequest("PUT", u, variable)
+	req, err := s.client.NewRequest("PUT", u, BuildVariable{key, value})
 	if err != nil {
 		return nil, nil, err
 	}
 
-	updatedVariable := new(Variable)
-	resp, err := s.client.Do(req, updatedVariable)
+	v := new(BuildVariable)
+	resp, err := s.client.Do(req, v)
 	if err != nil {
 		return nil, resp, err
 	}
 
-	return updatedVariable, resp, err
+	return v, resp, err
 }
 
-// RemoveVariable removes a project variable of a given project identified by its key
+// RemoveBuildVariable removes a project variable of a given project identified by its key
 //
 // Gitlab API Docs:
 // https://docs.gitlab.com/ce/api/build_variables.html#remove-variable
-func (s *VariablesService) RemoveVariable(pid interface{}, variableKey string) (*Response, error) {
+func (s *BuildVariablesService) RemoveBuildVariable(pid interface{}, key string) (*Response, error) {
 	project, err := parseID(pid)
 	if err != nil {
 		return nil, err
 	}
-
-	u := fmt.Sprintf("projects/%s/variables/%s", url.QueryEscape(project), variableKey)
+	u := fmt.Sprintf("projects/%s/variables/%s", url.QueryEscape(project), key)
 
 	req, err := s.client.NewRequest("DELETE", u, nil)
 	if err != nil {

variables_test.go → build_variables_test.go

@@ -7,92 +7,113 @@ import (
 	"testing"
 )
 
-func TestListVariables(t *testing.T) {
+const (
+	myKey      = "MY_KEY"
+	myValue    = "MY_VALUE"
+	myKey2     = "MY_KEY2"
+	myValue2   = "MY_VALUE2"
+	myNewValue = "MY_NEW_VALUE"
+)
+
+func TestListBuildVariables(t *testing.T) {
 	mux, server, client := setup()
 	defer teardown(server)
 
 	mux.HandleFunc("/projects/1/variables", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-		fmt.Fprint(w, `[{"key":"MY_KEY", "value": "MY_VALUE"},{"key":"MY_KEY2", "value": "MY_VALUE2"}]`)
+		fmt.Fprintf(w,
+			`[{"key":"%s","value":"%s"},{"key":"%s","value":"%s"}]`, myKey, myValue, myKey2, myValue2)
 	})
 
-	variables, _, err := client.Variables.ListVariables(1)
-
+	variables, _, err := client.BuildVariables.ListBuildVariables(1)
 	if err != nil {
-		t.Errorf("Projects.ListVariables returned error: %v", err)
+		t.Errorf("ListBuildVariables returned error: %v", err)
 	}
 
-	want := []*Variable{{Key: "MY_KEY", Value: "MY_VALUE"}, {Key: "MY_KEY2", Value: "MY_VALUE2"}}
+	want := []*BuildVariable{{Key: myKey, Value: myValue}, {Key: myKey2, Value: myValue2}}
 	if !reflect.DeepEqual(want, variables) {
-		t.Errorf("Projects.ListVariables returned %+v, want %+v", variables, want)
+		t.Errorf("ListBuildVariables returned %+v, want %+v", variables, want)
 	}
 }
 
-func TestGetSingleVariable(t *testing.T) {
+func TestGetBuildVariable(t *testing.T) {
 	mux, server, client := setup()
 	defer teardown(server)
 
-	mux.HandleFunc("/projects/1/variables/MY_KEY", func(w http.ResponseWriter, r *http.Request) {
+	mux.HandleFunc("/projects/1/variables/"+myKey, func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
-		fmt.Fprint(w, `{"key":"MY_KEY", "value": "MY_VALUE"}`)
+		fmt.Fprintf(w, `{"key":"%s","value":"%s"}`, myKey, myValue)
 	})
 
-	variable, _, err := client.Variables.GetSingleVariable(1, "MY_KEY")
-
+	variable, _, err := client.BuildVariables.GetBuildVariable(1, myKey)
 	if err != nil {
-		t.Errorf("Projects.GetSingleVariable returned error: %v", err)
+		t.Errorf("GetBuildVariable returned error: %v", err)
 	}
 
-	want := &Variable{Key: "MY_KEY", Value: "MY_VALUE"}
+	want := &BuildVariable{Key: myKey, Value: myValue}
 	if !reflect.DeepEqual(want, variable) {
-		t.Errorf("Projects.ListVariables returned %+v, want %+v", variable, want)
+		t.Errorf("GetBuildVariable returned %+v, want %+v", variable, want)
 	}
 }
 
-func testCreateVariable(t *testing.T) {
+func TestCreateBuildVariable(t *testing.T) {
 	mux, server, client := setup()
 	defer teardown(server)
 
 	mux.HandleFunc("/projects/1/variables", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "POST")
-		testJsonBody(t, r, values{
-			"key":   "MY_KEY",
-			"value": "MY_VALUE",
+		testJSONBody(t, r, values{
+			"key":   myKey,
+			"value": myValue,
 		})
-		fmt.Fprint(w, `{"key":"MY_KEY", "value": "MY_VALUE"}`)
+		fmt.Fprintf(w, `{"key":"%s","value":"%s"}`, myKey, myValue)
 	})
-	want := &Variable{Key: "MY_KEY", Value: "MY_VALUE"}
-	variable, _, err := client.Variables.CreateVariable(1, *want)
 
+	variable, _, err := client.BuildVariables.CreateBuildVariable(1, myKey, myValue)
 	if err != nil {
-		t.Errorf("Projects.CreateVariable returned error: %v", err)
+		t.Errorf("CreateBuildVariable returned error: %v", err)
 	}
 
+	want := &BuildVariable{Key: myKey, Value: myValue}
 	if !reflect.DeepEqual(want, variable) {
-		t.Errorf("Projects.CreateVariable returned %+v, want %+v", variable, want)
+		t.Errorf("CreateBuildVariable returned %+v, want %+v", variable, want)
 	}
 }
 
-func testUpdateVariable(t *testing.T) {
+func TestUpdateBuildVariable(t *testing.T) {
 	mux, server, client := setup()
 	defer teardown(server)
 
-	mux.HandleFunc("/projects/1/variables/MY_KEY", func(w http.ResponseWriter, r *http.Request) {
+	mux.HandleFunc("/projects/1/variables/"+myKey, func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "PUT")
-		testJsonBody(t, r, values{
-			"key":   "MY_KEY",
-			"value": "MY_NEW_VALUE",
+		testJSONBody(t, r, values{
+			"key":   myKey,
+			"value": myNewValue,
 		})
-		fmt.Fprint(w, `{"key":"MY_KEY", "value": "MY_NEW_VALUE"}`)
+		fmt.Fprintf(w, `{"key":"%s","value":"%s"}`, myKey, myNewValue)
 	})
-	want := &Variable{Key: "MY_KEY", Value: "MY_NEW_VALUE"}
-	variable, _, err := client.Variables.UpdateVariable(1, "MY_KEY", *want)
 
+	variable, _, err := client.BuildVariables.UpdateBuildVariable(1, myKey, myNewValue)
 	if err != nil {
-		t.Errorf("Projects.UpdateVariable returned error: %v", err)
+		t.Errorf("UpdateBuildVariable returned error: %v", err)
 	}
 
+	want := &BuildVariable{Key: myKey, Value: myNewValue}
 	if !reflect.DeepEqual(want, variable) {
-		t.Errorf("Projects.UpdateVariable returned %+v, want %+v", variable, want)
+		t.Errorf("UpdateBuildVariable returned %+v, want %+v", variable, want)
+	}
+}
+
+func TestRemoveBuildVariable(t *testing.T) {
+	mux, server, client := setup()
+	defer teardown(server)
+
+	mux.HandleFunc("/projects/1/variables/"+myKey, func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "DELETE")
+	})
+
+	_, err := client.BuildVariables.RemoveBuildVariable(1, myKey)
+	if err != nil {
+		t.Errorf("RemoveBuildVariable returned error: %v", err)
 	}
 }

builds.go

@@ -120,11 +120,11 @@ func (s *BuildsService) ListCommitBuilds(pid interface{}, sha string, opts *List
 	return builds, resp, err
 }
 
-// GetSingleBuild gets a single build of a project.
+// GetBuild gets a single build of a project.
 //
 // GitLab API docs:
 // https://docs.gitlab.com/ce/api/builds.html#get-a-single-build
-func (s *BuildsService) GetSingleBuild(pid interface{}, buildID int) (*Build, *Response, error) {
+func (s *BuildsService) GetBuild(pid interface{}, buildID int) (*Build, *Response, error) {
 	project, err := parseID(pid)
 	if err != nil {
 		return nil, nil, err

commits.go

@@ -169,6 +169,7 @@ type CommitComment struct {
 	Author   Author `json:"author"`
 }
 
+// Author represents a GitLab commit author
 type Author struct {
 	ID        int        `json:"id"`
 	Username  string     `json:"username"`
@@ -271,7 +272,7 @@ type CommitStatus struct {
 	Ref         string     `json:"ref"`
 	Status      string     `json:"status"`
 	Name        string     `json:"name"`
-	TargetUrl   string     `json:"target_url"`
+	TargetURL   string     `json:"target_url"`
 	Description string     `json:"description"`
 	CreatedAt   *time.Time `json:"created_at"`
 	StartedAt   *time.Time `json:"started_at"`
@@ -314,12 +315,14 @@ type SetCommitStatusOptions struct {
 	Ref         *string    `url:"ref,omitempty" json:"ref,omitempty"`
 	Name        *string    `url:"name,omitempty" json:"name,omitempty"`
 	Context     *string    `url:"context,omitempty" json:"context,omitempty"`
-	TargetUrl   *string    `url:"target_url,omitempty" json:"target_url,omitempty"`
+	TargetURL   *string    `url:"target_url,omitempty" json:"target_url,omitempty"`
 	Description *string    `url:"description,omitempty" json:"description,omitempty"`
 }
 
+// BuildState represents a GitLab build state
 type BuildState string
 
+// These constants represent all valid build states
 const (
 	Pending  BuildState = "pending"
 	Running  BuildState = "running"

commits_test.go

@@ -41,7 +41,7 @@ func TestSetCommitStatus(t *testing.T) {
 
 	mux.HandleFunc("/projects/1/statuses/b0b3a907f41409829b307a28b82fdbd552ee5a27", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "POST")
-		testJsonBody(t, r, values{
+		testJSONBody(t, r, values{
 			"state":       "running",
 			"context":     "",
 			"ref":         "master",

gitlab.go

@@ -116,6 +116,7 @@ type Client struct {
 
 	// Services used for talking to different parts of the GitLab API.
 	Branches        *BranchesService
+	BuildVariables  *BuildVariablesService
 	Builds          *BuildsService
 	Commits         *CommitsService
 	DeployKeys      *DeployKeysService
@@ -136,7 +137,6 @@ type Client struct {
 	SystemHooks     *SystemHooksService
 	Tags            *TagsService
 	Users           *UsersService
-	Variables       *VariablesService
 }
 
 // ListOptions specifies the optional parameters to various List methods that
@@ -175,6 +175,7 @@ func newClient(httpClient *http.Client, tokenType tokenType, token string) *Clie
 	}
 
 	c.Branches = &BranchesService{client: c}
+	c.BuildVariables = &BuildVariablesService{client: c}
 	c.Builds = &BuildsService{client: c}
 	c.Commits = &CommitsService{client: c}
 	c.DeployKeys = &DeployKeysService{client: c}
@@ -195,7 +196,6 @@ func newClient(httpClient *http.Client, tokenType tokenType, token string) *Clie
 	c.SystemHooks = &SystemHooksService{client: c}
 	c.Tags = &TagsService{client: c}
 	c.Users = &UsersService{client: c}
-	c.Variables = &VariablesService{client: c}
 
 	return c
 }
@@ -235,7 +235,6 @@ func (c *Client) NewRequest(method, path string, opt interface{}) (*http.Request
 			return nil, err
 		}
 		u.RawQuery = q.Encode()
-
 	}
 
 	req := &http.Request{

gitlab_test.go

@@ -33,7 +33,7 @@ func teardown(server *httptest.Server) {
 	server.Close()
 }
 
-func testUrl(t *testing.T, r *http.Request, want string) {
+func testURL(t *testing.T, r *http.Request, want string) {
 	if got := r.RequestURI; got != want {
 		t.Errorf("Request url: %+v, want %s", got, want)
 	}
@@ -53,7 +53,11 @@ func testFormValues(t *testing.T, r *http.Request, values values) {
 		want.Add(k, v)
 	}
 
-	r.ParseForm()
+	err := r.ParseForm()
+	if err != nil {
+		t.Errorf("Error parsing form: %v", err)
+	}
+
 	if got := r.Form; !reflect.DeepEqual(got, want) {
 		t.Errorf("Request parameters: %v, want %v", got, want)
 	}
@@ -70,19 +74,23 @@ func testBody(t *testing.T, r *http.Request, want string) {
 	if err != nil {
 		t.Errorf("Error reading request body: %v", err)
 	}
+
 	if got := string(b); got != want {
 		t.Errorf("request Body is %s, want %s", got, want)
 	}
 }
 
-func testJsonBody(t *testing.T, r *http.Request, want values) {
+func testJSONBody(t *testing.T, r *http.Request, want values) {
 	b, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		t.Errorf("Error reading request body: %v", err)
 	}
 
 	var got values
-	json.Unmarshal(b, &got)
+	err = json.Unmarshal(b, &got)
+	if err != nil {
+		t.Errorf("Error unmarshalling request body: %v", err)
+	}
 
 	if !reflect.DeepEqual(got, want) {
 		t.Errorf("Request parameters: %v, want %v", got, want)

projects_test.go

@@ -161,7 +161,7 @@ func TestGetProject_byName(t *testing.T) {
 	defer teardown(server)
 
 	mux.HandleFunc("/projects/", func(w http.ResponseWriter, r *http.Request) {
-		testUrl(t, r, "/projects/namespace%2Fname")
+		testURL(t, r, "/projects/namespace%2Fname")
 		testMethod(t, r, "GET")
 		fmt.Fprint(w, `{"id":1}`)
 	})
@@ -212,7 +212,7 @@ func TestCreateProject(t *testing.T) {
 
 	mux.HandleFunc("/projects", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "POST")
-		testJsonBody(t, r, values{
+		testJSONBody(t, r, values{
 			"name": "n",
 		})
 

services.go

@@ -30,6 +30,9 @@ type ServicesService struct {
 	client *Client
 }
 
+// Service represents a GitLab service.
+//
+// GitLab API docs: https://docs.gitlab.com/ce/api/services.html
 type Service struct {
 	ID                  *int       `json:"id"`
 	Title               *string    `json:"title"`

services_test.go

@@ -13,7 +13,7 @@ func TestSetDroneCIService(t *testing.T) {
 
 	mux.HandleFunc("/projects/1/services/drone-ci", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "PUT")
-		testJsonBody(t, r, values{
+		testJSONBody(t, r, values{
 			"token":                   "t",
 			"drone_url":               "u",
 			"enable_ssl_verification": "true",

tags.go

@@ -68,12 +68,12 @@ func (s *TagsService) ListTags(pid interface{}) ([]*Tag, *Response, error) {
 	return t, resp, err
 }
 
-// Get a specific repository tag determined by its name. It returns 200 together
+// GetTag a specific repository tag determined by its name. It returns 200 together
 // with the tag information if the tag exists. It returns 404 if the tag does not exist.
 //
 // GitLab API docs:
 // https://docs.gitlab.com/ce/api/tags.html#get-a-single-repository-tag
-func (s *TagsService) GetSingleTag(pid interface{}, tag string) (*Tag, *Response, error) {
+func (s *TagsService) GetTag(pid interface{}, tag string) (*Tag, *Response, error) {
 	project, err := parseID(pid)
 	if err != nil {
 		return nil, nil, err