Merge pull request lgtmco#10 from jonbodner/refactoring

Refactoring -- Format code properly and remove unused code

approval/approval.go

@@ -1,11 +1,11 @@
 package approval
 
 import (
+	"fmt"
+	log "github.com/Sirupsen/logrus"
 	"github.com/lgtmco/lgtm/model"
 	"regexp"
 	"strings"
-	"fmt"
-	log "github.com/Sirupsen/logrus"
 )
 
 // Func takes in the information needed to figure out which approvers were in the PR comments
@@ -19,13 +19,13 @@ func Register(name string, f Func) error {
 		return fmt.Errorf("Approval Algorithm %s is already registered.", name)
 	}
 	approvalMap[strings.ToLower(name)] = f
-	log.Debug("added to approvalMap:",name,f)
+	log.Debug("added to approvalMap:", name, f)
 	return nil
 }
 
 func Lookup(name string) (Func, error) {
-	log.Debug("approvalMap has",approvalMap)
-	log.Debugf("looking for '%s'\n",name)
+	log.Debug("approvalMap has", approvalMap)
+	log.Debugf("looking for '%s'\n", name)
 	f, ok := approvalMap[strings.ToLower(name)]
 	if !ok {
 		return nil, fmt.Errorf("Unknown Approval Algorithm %s", name)

approval/org/org.go

@@ -1,9 +1,9 @@
 package org
 
 import (
-	"regexp"
-	"github.com/lgtmco/lgtm/model"
 	"github.com/lgtmco/lgtm/approval"
+	"github.com/lgtmco/lgtm/model"
+	"regexp"
 )
 
 func init() {

approval/org/org_test.go

@@ -1,8 +1,8 @@
 package org
 
 import (
-	"testing"
 	"github.com/lgtmco/lgtm/model"
+	"testing"
 )
 
 var maintainerToml = `
@@ -34,8 +34,8 @@ people = [
 `
 
 func TestOrg(t *testing.T) {
-	config := &model.Config {
-		Pattern: `(?i)LGTM\s*(\S*)`,
+	config := &model.Config{
+		Pattern:         `(?i)LGTM\s*(\S*)`,
 		SelfApprovalOff: true,
 	}
 	m, err := model.ParseMaintainerStr(maintainerToml)
@@ -45,25 +45,25 @@ func TestOrg(t *testing.T) {
 	issue := &model.Issue{
 		Author: "jon",
 	}
-	comments := []*model.Comment {
+	comments := []*model.Comment{
 		{
-			Body: "lgtm",
+			Body:   "lgtm",
 			Author: "bob",
 		},
 		{
-			Body: "lgtm",
+			Body:   "lgtm",
 			Author: "qwerty",
 		},
 		{
-			Body: "not an approval",
+			Body:   "not an approval",
 			Author: "ralph",
 		},
 		{
-			Body: "lgtm",
+			Body:   "lgtm",
 			Author: "george",
 		},
 		{
-			Body: "lgtm",
+			Body:   "lgtm",
 			Author: "ralph",
 		},
 	}
@@ -77,4 +77,4 @@ func TestOrg(t *testing.T) {
 	if people[0] != "george" {
 		t.Errorf("Expected george, had %s", people[0])
 	}
-}
+}

model/branch_status.go

@@ -1,3 +1,3 @@
 package model
 
-type BranchStatus string
+type BranchStatus string

model/status_hook.go

@@ -1,8 +1,8 @@
 package model
 
 type StatusHook struct {
-	SHA string
-	Repo    *Repo
+	SHA  string
+	Repo *Repo
 }
 
 type PullRequest struct {
@@ -19,10 +19,10 @@ type Branch struct {
 type PRHook struct {
 	Number int
 	Update bool
-	Repo    *Repo
+	Repo   *Repo
 }
 
 type PushHook struct {
-	SHA string
+	SHA  string
 	Repo *Repo
-}
+}

remote/github/github.go

@@ -7,12 +7,12 @@ import (
 	"strings"
 	"time"
 
+	"errors"
 	log "github.com/Sirupsen/logrus"
 	"github.com/google/go-github/github"
 	"github.com/lgtmco/lgtm/model"
 	"github.com/lgtmco/lgtm/shared/httputil"
 	"golang.org/x/oauth2"
-	"errors"
 )
 
 // name of the status message posted to GitHub
@@ -204,7 +204,7 @@ func (g *Github) SetHook(user *model.User, repo *model.Repo, link string) error
 	/*
 		JCB 04/21/16 confirmed with Github support -- must specify all existing contexts when
 		 adding a new one, otherwise the other contexts will be removed.
-	 */
+	*/
 	client_ := NewClientToken(g.API, user.Token)
 	branch, _ := client_.Branch(repo.Owner, repo.Name, *repo_.DefaultBranch)
 
@@ -440,65 +440,6 @@ func (g *Github) GetPRHook(r *http.Request) (*model.PRHook, error) {
 	return hook, nil
 }
 
-func (g *Github) GetPushHook(r *http.Request) (*model.PushHook, error) {
-
-	// only process comment hooks
-	if r.Header.Get("X-Github-Event") != "push" {
-		return nil, nil
-	}
-
-	data := pushHook{}
-	err := json.NewDecoder(r.Body).Decode(&data)
-	if err != nil {
-		return nil, err
-	}
-
-	log.Debug(data)
-
-	if data.HeadCommit == nil {
-		return nil, nil
-	}
-
-	hook := new(model.PushHook)
-
-	hook.SHA = data.HeadCommit.ID
-	hook.Repo = new(model.Repo)
-	hook.Repo.Owner = data.Repository.Owner.Login
-	hook.Repo.Name = data.Repository.Name
-	hook.Repo.Slug = data.Repository.FullName
-
-	log.Debug(*hook)
-
-	return hook, nil
-}
-
-func (g *Github) UpdatePRsForCommit(u *model.User, r *model.Repo, sha *string) (bool, error) {
-	//if we have a new commit on an existing open PR, then we need to put a new pending LGTM status on it
-	//if the commit is not associated with an open PR, then we don't do any status update
-	client := setupClient(g.API, u.Token)
-	log.Debug("sha == ", *sha)
-	issues, _, err := client.Search.Issues(fmt.Sprintf("%s&type=pr", *sha), &github.SearchOptions{
-		TextMatch: false,
-	})
-	if err != nil {
-		return false, err
-	}
-	if issues.Total != nil && *issues.Total > 0 {
-		status := "pending"
-		desc := "this commit is pending approval"
-
-		data := github.RepoStatus{
-			Context:     github.String(context),
-			State:       github.String(status),
-			Description: github.String(desc),
-		}
-
-		_, _, err = client.Repositories.CreateStatus(r.Owner, r.Name, *sha, &data)
-		return true, err
-	}
-	return false, nil
-}
-
 func (g *Github) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *string) ([]model.PullRequest, error) {
 	client := setupClient(g.API, u.Token)
 	log.Debug("sha == ", *sha)
@@ -549,30 +490,19 @@ func (g *Github) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *str
 		out = append(out, model.PullRequest{
 			Issue: model.Issue{
 				Number: *v.Number,
-				Title: *v.Title,
+				Title:  *v.Title,
 				Author: *v.User.Login,
 			},
-			Branch:  model.Branch{
-				Name: *pr.Head.Ref,
+			Branch: model.Branch{
+				Name:         *pr.Head.Ref,
 				BranchStatus: combinedState,
-				Mergeable: mergeable,
-
+				Mergeable:    mergeable,
 			},
 		})
 	}
 	return out, nil
 }
 
-func (g *Github) GetBranchStatus(u *model.User, r *model.Repo, branch string) (*model.BranchStatus, error) {
-	client := setupClient(g.API, u.Token)
-	statuses, _, err := client.Repositories.GetCombinedStatus(r.Owner, r.Name, branch, nil)
-	if err != nil {
-		return nil, err
-	}
-
-	return (*model.BranchStatus)(statuses.State), nil
-}
-
 func (g *Github) MergePR(u *model.User, r *model.Repo, pullRequest model.PullRequest, approvers []*model.Person) (*string, error) {
 	client := setupClient(g.API, u.Token)
 
@@ -651,7 +581,6 @@ func (g *Github) Tag(u *model.User, r *model.Repo, version *string, sha *string)
 	return err
 }
 
-
 func (g *Github) WriteComment(u *model.User, r *model.Repo, num int, message string) error {
 	client := setupClient(g.API, u.Token)
 	emsg := "Message from LGTM -- " + message

remote/github/types.go

@@ -42,43 +42,43 @@ type commentHook struct {
 }
 
 type statusHook struct {
-	SHA string `json:"sha"`
+	SHA   string `json:"sha"`
 	State string `json:"state"`
 
 	Branches []struct {
-		Name string `json:"name"`
+		Name   string `json:"name"`
 		Commit struct {
 			SHA string `json:"sha"`
 			URL string `json:"url"`
-		       } `json:"commit"`
+		} `json:"commit"`
 	} `json:"branches"`
 
 	Repository Repository `json:"repository"`
 }
 
 type prHook struct {
 	Action string `json:"action"`
-	Number int `json:"number"`
+	Number int    `json:"number"`
 
 	Repository Repository `json:"repository"`
 }
 
 type pushHook struct {
 	HeadCommit *struct {
-			ID string `json:"id"`
-		   }  `json:"head_commit"`
+		ID string `json:"id"`
+	} `json:"head_commit"`
 
 	Repository Repository `json:"repository"`
 }
 
-type 	Repository struct {
+type Repository struct {
 	Name     string `json:"name"`
 	FullName string `json:"full_name"`
 	Desc     string `json:"description"`
 	Private  bool   `json:"private"`
 	Owner    struct {
-			 Login  string `json:"login"`
-			 Type   string `json:"type"`
-			 Avatar string `json:"avatar_url"`
-		 } `json:"owner"`
+		Login  string `json:"login"`
+		Type   string `json:"type"`
+		Avatar string `json:"avatar_url"`
+	} `json:"owner"`
 }

remote/mock/remote.go

@@ -27,29 +27,6 @@ func (_m *Remote) DelHook(_a0 *model.User, _a1 *model.Repo, _a2 string) error {
 	return r0
 }
 
-// GetBranchStatus provides a mock function with given fields: _a0, _a1, _a2
-func (_m *Remote) GetBranchStatus(_a0 *model.User, _a1 *model.Repo, _a2 string) (*model.BranchStatus, error) {
-	ret := _m.Called(_a0, _a1, _a2)
-
-	var r0 *model.BranchStatus
-	if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, string) *model.BranchStatus); ok {
-		r0 = rf(_a0, _a1, _a2)
-	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(*model.BranchStatus)
-		}
-	}
-
-	var r1 error
-	if rf, ok := ret.Get(1).(func(*model.User, *model.Repo, string) error); ok {
-		r1 = rf(_a0, _a1, _a2)
-	} else {
-		r1 = ret.Error(1)
-	}
-
-	return r0, r1
-}
-
 // GetComments provides a mock function with given fields: _a0, _a1, _a2
 func (_m *Remote) GetComments(_a0 *model.User, _a1 *model.Repo, _a2 int) ([]*model.Comment, error) {
 	ret := _m.Called(_a0, _a1, _a2)