diff --git a/approval/approval.go b/approval/approval.go index c1b8d85..7da2de5 100644 --- a/approval/approval.go +++ b/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) diff --git a/approval/org/org.go b/approval/org/org.go index ae1c237..eb4eb07 100644 --- a/approval/org/org.go +++ b/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() { diff --git a/approval/org/org_test.go b/approval/org/org_test.go index 3f4c2e1..ecb59e5 100644 --- a/approval/org/org_test.go +++ b/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]) } -} \ No newline at end of file +} diff --git a/model/branch_status.go b/model/branch_status.go index 51f3824..452f937 100644 --- a/model/branch_status.go +++ b/model/branch_status.go @@ -1,3 +1,3 @@ package model -type BranchStatus string \ No newline at end of file +type BranchStatus string diff --git a/model/status_hook.go b/model/status_hook.go index 9a2e571..63258f2 100644 --- a/model/status_hook.go +++ b/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 -} \ No newline at end of file +} diff --git a/remote/github/github.go b/remote/github/github.go index 47bccae..98b38a0 100644 --- a/remote/github/github.go +++ b/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 diff --git a/remote/github/types.go b/remote/github/types.go index d234c55..6cc1af0 100644 --- a/remote/github/types.go +++ b/remote/github/types.go @@ -42,15 +42,15 @@ 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"` @@ -58,27 +58,27 @@ type statusHook struct { 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"` } diff --git a/remote/mock/remote.go b/remote/mock/remote.go index 33f92f4..9b36534 100644 --- a/remote/mock/remote.go +++ b/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) diff --git a/remote/remote.go b/remote/remote.go index d564ef2..19317ec 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -59,12 +59,6 @@ type Remote interface { // GetPRHook gets the pull request hook from the http Request. GetPRHook(r *http.Request) (*model.PRHook, error) - // GetPushHook gets the push hook from the http Request. - GetPushHook(r *http.Request) (*model.PushHook, error) - - // GetBranchStatus returns overall status for the named branch from the remote system - GetBranchStatus(*model.User, *model.Repo, string) (*model.BranchStatus, error) - // MergePR merges the named pull request from the remote system MergePR(u *model.User, r *model.Repo, pullRequest model.PullRequest, approvers []*model.Person) (*string, error) @@ -77,9 +71,6 @@ type Remote interface { // GetPullRequestsForCommit returns all pull requests associated with a commit SHA GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *string) ([]model.PullRequest, error) - // UpdatePRsForCommit sets the commit's status to pending for LGTM if it is already on an open Pull Request - UpdatePRsForCommit(u *model.User, r *model.Repo, sha *string) (bool, error) - // WriteComment puts a new comment from LGTM into the PR WriteComment(u *model.User, r *model.Repo, num int, message string) error } @@ -165,16 +156,6 @@ func GetPRHook(c context.Context, r *http.Request) (*model.PRHook, error) { return FromContext(c).GetPRHook(r) } -// GetPushHook gets the push hook from the http Request. -func GetPushHook(c context.Context, r *http.Request) (*model.PushHook, error) { - return FromContext(c).GetPushHook(r) -} - -// GetBranchStatus gets the overal status for a branch from the remote repository. -func GetBranchStatus(c context.Context, u *model.User, r *model.Repo, branch string) (*model.BranchStatus, error) { - return FromContext(c).GetBranchStatus(u, r, branch) -} - func MergePR(c context.Context, u *model.User, r *model.Repo, pullRequest model.PullRequest, approvers []*model.Person) (*string, error) { return FromContext(c).MergePR(u, r, pullRequest, approvers) } @@ -192,10 +173,6 @@ func GetPullRequestsForCommit(c context.Context, u *model.User, r *model.Repo, s } -func UpdatePRsForCommit(c context.Context, u *model.User, r *model.Repo, sha *string) (bool, error) { - return FromContext(c).UpdatePRsForCommit(u, r, sha) -} - func WriteComment(c context.Context, u *model.User, r *model.Repo, num int, message string) error { return FromContext(c).WriteComment(u, r, num, message) -} \ No newline at end of file +} diff --git a/web/comment_hook.go b/web/comment_hook.go index f10960e..8e42de3 100644 --- a/web/comment_hook.go +++ b/web/comment_hook.go @@ -1,11 +1,11 @@ package web import ( - "github.com/gin-gonic/gin" log "github.com/Sirupsen/logrus" + "github.com/gin-gonic/gin" + "github.com/lgtmco/lgtm/approval" "github.com/lgtmco/lgtm/model" "github.com/lgtmco/lgtm/remote" - "github.com/lgtmco/lgtm/approval" ) func processCommentHook(c *gin.Context, hook *model.Hook) { diff --git a/web/hook.go b/web/hook.go index be54ca5..a8c7446 100644 --- a/web/hook.go +++ b/web/hook.go @@ -1,11 +1,11 @@ package web import ( + log "github.com/Sirupsen/logrus" + "github.com/gin-gonic/gin" "github.com/lgtmco/lgtm/cache" "github.com/lgtmco/lgtm/model" "github.com/lgtmco/lgtm/remote" - log "github.com/Sirupsen/logrus" - "github.com/gin-gonic/gin" "github.com/lgtmco/lgtm/store" ) @@ -26,7 +26,7 @@ func Hook(c *gin.Context) { c.String(500, "Error parsing status hook. %s", err) return } - if statusHook != nil { + if statusHook != nil { processStatusHook(c, statusHook) } @@ -76,11 +76,11 @@ func processPRHook(c *gin.Context, prHook *model.PRHook) { c.IndentedJSON(200, gin.H{ "number": prHook.Number, - "approved": false, + "approved": false, }) } -func getRepoAndUser(c *gin.Context, slug string) (*model.Repo, *model.User, error){ +func getRepoAndUser(c *gin.Context, slug string) (*model.Repo, *model.User, error) { repo, err := store.GetRepoSlug(c, slug) if err != nil { log.Errorf("Error getting repository %s. %s", slug, err) @@ -141,4 +141,3 @@ func getComments(c *gin.Context, user *model.User, repo *model.Repo, num int) ([ } return comments, nil } - diff --git a/web/push_hook.go b/web/push_hook.go deleted file mode 100644 index f2d9d1e..0000000 --- a/web/push_hook.go +++ /dev/null @@ -1,29 +0,0 @@ -package web - -import ( - log "github.com/Sirupsen/logrus" - "github.com/gin-gonic/gin" - "github.com/lgtmco/lgtm/model" - "github.com/lgtmco/lgtm/remote" -) - -func processPushHook(c *gin.Context, pushHook *model.PushHook) { - repo, user, err := getRepoAndUser(c, pushHook.Repo.Slug) - if err != nil { - return - } - updated, err := remote.UpdatePRsForCommit(c, user, repo, &pushHook.SHA) - if err != nil { - log.Errorf("Error setting status. %s", err) - c.String(500, "Error setting status. %s", err) - return - } - if updated { - c.IndentedJSON(200, gin.H{ - "commit": pushHook.SHA, - "status": "pending", - }) - } else { - c.IndentedJSON(200, gin.H{}) - } -} diff --git a/web/status_hook.go b/web/status_hook.go index 6283ba8..93d7beb 100644 --- a/web/status_hook.go +++ b/web/status_hook.go @@ -154,7 +154,7 @@ func handleSemver(c *gin.Context, user *model.User, hook *model.StatusHook, pr m maxVer = foundVersion } else { maxParts := maxVer.Segments() - maxVer, _ = version.NewVersion(fmt.Sprintf("%d.%d.%d", maxParts[0], maxParts[1], maxParts[2] + 1)) + maxVer, _ = version.NewVersion(fmt.Sprintf("%d.%d.%d", maxParts[0], maxParts[1], maxParts[2]+1)) } verStr := maxVer.String() @@ -187,7 +187,7 @@ func getMaxExistingTag(tags []model.Tag) *version.Version { // the function returns version 0.0.0. If there's a bug in the version pattern, // nil will be returned. func getMaxVersionComment(config *model.Config, maintainer *model.Maintainer, -issue model.Issue, comments []*model.Comment, matcher approval.Func) *version.Version { + issue model.Issue, comments []*model.Comment, matcher approval.Func) *version.Version { maxVersion, _ := version.NewVersion("0.0.0") ma, err := regexp.Compile(config.Pattern) if err != nil { diff --git a/web/status_hook_test.go b/web/status_hook_test.go index 55a8f2d..93faceb 100644 --- a/web/status_hook_test.go +++ b/web/status_hook_test.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/gin-gonic/gin" "github.com/hashicorp/go-version" + "github.com/lgtmco/lgtm/approval" "github.com/lgtmco/lgtm/model" "github.com/lgtmco/lgtm/remote" "math" @@ -12,7 +13,6 @@ import ( "strconv" "testing" "time" - "github.com/lgtmco/lgtm/approval" ) func TestHandleTimestampMillis(t *testing.T) { @@ -28,7 +28,7 @@ func TestHandleTimestampMillis(t *testing.T) { if err != nil { t.Error(err) } - if math.Abs(float64(m - m1)) > 100 { + if math.Abs(float64(m-m1)) > 100 { t.Errorf("shouldn't be that different: %d, %d", m, m1) } } @@ -350,8 +350,8 @@ func TestHandleSemver(t *testing.T) { remote.ToContext(c, &myR{}) config := &model.Config{ - DoVersion: true, - Pattern: `(?i)LGTM\s*(\S*)`, + DoVersion: true, + Pattern: `(?i)LGTM\s*(\S*)`, ApprovalAlg: "simple", } m := &model.Maintainer{ @@ -436,8 +436,8 @@ func TestHandleSemver2(t *testing.T) { remote.ToContext(c, &myR2{}) config := &model.Config{ - DoVersion: true, - Pattern: `(?i)LGTM\s*(\S*)`, + DoVersion: true, + Pattern: `(?i)LGTM\s*(\S*)`, ApprovalAlg: "simple", } m := &model.Maintainer{ @@ -518,8 +518,8 @@ func TestHandleSemver3(t *testing.T) { remote.ToContext(c, &myR3{}) config := &model.Config{ - DoVersion: true, - Pattern: `(?i)LGTM\s*(\S*)`, + DoVersion: true, + Pattern: `(?i)LGTM\s*(\S*)`, ApprovalAlg: "simple", } m := &model.Maintainer{ @@ -593,8 +593,8 @@ func TestHandleSemver4(t *testing.T) { remote.ToContext(c, &myR4{}) config := &model.Config{ - DoVersion: true, - Pattern: `(?i)LGTM\s*(\S*)`, + DoVersion: true, + Pattern: `(?i)LGTM\s*(\S*)`, ApprovalAlg: "simple", } m := &model.Maintainer{