Skip to content

Commit

Permalink
Merge pull request #1 from jonbodner/cap_one_approval_policy
Browse files Browse the repository at this point in the history
Factor approval code, add org validation
  • Loading branch information
Jon Bodner authored and Jon Bodner committed May 5, 2016
2 parents aa873ee + d00feae commit aa3adad
Show file tree
Hide file tree
Showing 5 changed files with 243 additions and 174 deletions.
25 changes: 11 additions & 14 deletions approval/approval.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ import (

// Func takes in the information needed to figure out which approvers were in the PR comments
// and returns a slice of the approvers that were found
type Func func (*model.Config, *model.Maintainer, *model.Issue, []*model.Comment) []*model.Person
type Func func(*model.Config, *model.Maintainer, *model.Issue, []*model.Comment, Processor)

var approvalMap = map[string]Func{}

func Register(name string, f Func) error {
if _, ok := approvalMap[strings.ToLower(name)]; ok {
return fmt.Errorf("Approval Algorithm %s is already registered.",name)
return fmt.Errorf("Approval Algorithm %s is already registered.", name)
}
approvalMap[strings.ToLower(name)] = f
return nil
Expand All @@ -24,7 +24,7 @@ func Register(name string, f Func) error {
func Lookup(name string) (Func, error) {
f, ok := approvalMap[strings.ToLower(name)]
if !ok {
return nil, fmt.Errorf("Unknown Approval Algorithm %s",name)
return nil, fmt.Errorf("Unknown Approval Algorithm %s", name)
}
return f, nil
}
Expand All @@ -33,16 +33,17 @@ func init() {
Register("simple", Simple)
}

type Processor func(*model.Maintainer, *model.Comment)

// Simple is a helper function that analyzes the list of comments
// and returns the list of approvers.
func Simple(config *model.Config, maintainer *model.Maintainer, issue *model.Issue, comments []*model.Comment) []*model.Person {
// and finds the ones that have approvers on the maintainers list.
func Simple(config *model.Config, maintainer *model.Maintainer, issue *model.Issue, comments []*model.Comment, p Processor) {
approverm := map[string]bool{}
approvers := []*model.Person{}

matcher, err := regexp.Compile(config.Pattern)
if err != nil {
// this should never happen
return approvers
return
}

for _, comment := range comments {
Expand All @@ -51,7 +52,7 @@ func Simple(config *model.Config, maintainer *model.Maintainer, issue *model.Iss
continue
}
// the user must be a valid maintainer of the project
person, ok := maintainer.People[comment.Author]
_, ok := maintainer.People[comment.Author]
if !ok {
continue
}
Expand All @@ -62,11 +63,7 @@ func Simple(config *model.Config, maintainer *model.Maintainer, issue *model.Iss
// verify the comment matches the approval pattern
if matcher.MatchString(comment.Body) {
approverm[comment.Author] = true
approvers = append(approvers, person)
p(maintainer, comment)
}
}

return approvers
}


}
78 changes: 78 additions & 0 deletions approval/org/org.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package org

import (
"regexp"
"github.com/lgtmco/lgtm/model"
"github.com/lgtmco/lgtm/approval"
)

func init() {
approval.Register("org", Org)
}

// Org is a helper function that analyzes the list of comments
// and returns the list of approvers.
// The rules for Org are:
// - At most one approver from each team
// - If SelfApprovalOff is true, no member of the team of the creator of the Pull Request is allowed to approve the PR
// - If a person appears on more than one team, they only count once, for the first team in which they appear
// (not solving the optimal grouping problem yet)
func Org(config *model.Config, maintainer *model.Maintainer, issue *model.Issue, comments []*model.Comment, p approval.Processor) {
//groups that have already approved
approvergm := map[string]bool{}

//org that the author belongs to
authorOrg := ""

//key is person, value is map of the orgs for that person
orgMap := map[string]map[string]bool{}

//key is org name, value is org
for k, v := range maintainer.Org {
//value is name of person in the org
for _, name := range v.People {
if name == issue.Author {
authorOrg = name
}
m, ok := orgMap[name]
if !ok {
m := map[string]bool{}
orgMap[name] = m
}
m[k] = true
}
}

matcher, err := regexp.Compile(config.Pattern)
if err != nil {
// this should never happen
return
}

for _, comment := range comments {
// verify the comment matches the approval pattern
if !matcher.MatchString(comment.Body) {
continue
}
//get the orgs for the current comment's author
curOrgs, ok := orgMap[comment.Author]
if !ok {
// the user must be a valid maintainer of the project
continue
}
for curOrg := range curOrgs {
// your group cannot lgtm your own pull request
if config.SelfApprovalOff && curOrg == authorOrg {
continue
}
// your group cannot approve twice
if approvergm[curOrg] {
continue
}
//found one!
approvergm[curOrg] = true
p(maintainer, comment)
}
}
return
}
12 changes: 11 additions & 1 deletion web/comment_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ func processCommentHook(c *gin.Context, hook *model.Hook) {
c.String(500, "Error getting approval algorithm %s. %s", config.ApprovalAlg, err)
return
}
approvers := alg(config, maintainer, hook.Issue, comments)
approvers := getApprovers(config, maintainer, hook.Issue, comments, alg)

approved := len(approvers) >= config.Approvals
err = remote.SetStatus(c, user, repo, hook.Issue.Number, approved)
if err != nil {
Expand All @@ -48,3 +49,12 @@ func processCommentHook(c *gin.Context, hook *model.Hook) {
"approved_by": approvers,
})
}

func getApprovers(config *model.Config, maintainer *model.Maintainer,
issue *model.Issue, comments []*model.Comment, matcher approval.Func) []*model.Person {
approvers := []*model.Person{}
matcher(config, maintainer, issue, comments, func(maintainer *model.Maintainer, comment *model.Comment) {
approvers = append(approvers, maintainer.People[comment.Author])
})
return approvers
}
116 changes: 49 additions & 67 deletions web/status_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@ package web

import (
"fmt"
log "github.com/Sirupsen/logrus"
"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"
log "github.com/Sirupsen/logrus"
"github.com/hashicorp/go-version"
"regexp"
"time"
)
Expand All @@ -28,8 +29,7 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) {
}

if !config.DoMerge {
c.IndentedJSON(200, gin.H{
})
c.IndentedJSON(200, gin.H{})
return
}

Expand Down Expand Up @@ -75,9 +75,6 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) {
verStr, err = handleSemver(c, user, hook, v, config, maintainer, repo)

}
if config.VersionAlg == "semver" {
verStr, err = handleSemver(c, user, hook, v, config, maintainer, repo)
}
if err != nil {
continue
}
Expand All @@ -93,18 +90,18 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) {
log.Debugf("processed status for %s. received %v ", repo.Slug, hook)

c.IndentedJSON(200, gin.H{
"merged": merged,
"merged": merged,
})
}

func handleTimestamp(config *model.Config) (*string, error) {
/*
All times are in UTC
Valid format strings:
- A standard Go format string
- blank or rfc3339: RFC 3339 format
- millis: milliseconds since the epoch
*/
All times are in UTC
Valid format strings:
- A standard Go format string
- blank or rfc3339: RFC 3339 format
- millis: milliseconds since the epoch
*/
curTime := time.Now().UTC()
var format string
switch config.VersionFormat {
Expand All @@ -122,6 +119,11 @@ func handleTimestamp(config *model.Config) (*string, error) {
}

func handleSemver(c *gin.Context, user *model.User, hook *model.StatusHook, pr model.PullRequest, config *model.Config, maintainer *model.Maintainer, repo *model.Repo) (*string, error) {
alg, err := approval.Lookup(config.ApprovalAlg)
if err != nil {
log.Warnf("Error getting approval algorithm %s. %s", config.ApprovalAlg, err)
return nil, err
}
// to create the version, need to scan the comments on the pull request to see if anyone specified a version #
// if so, use the largest specified version #. if not, increment the last version version # for the release
tags, err := remote.ListTags(c, user, hook.Repo)
Expand All @@ -136,70 +138,19 @@ func handleSemver(c *gin.Context, user *model.User, hook *model.StatusHook, pr m
return nil, err
}

foundVersion := getMaxVersionComment(config, maintainer, pr.Issue, comments)
foundVersion := getMaxVersionComment(config, maintainer, pr.Issue, comments, alg)

if foundVersion != nil && foundVersion.GreaterThan(maxVer) {
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()
return &verStr, nil
}

// getMaxVersionComment is a helper function that analyzes the list of comments
// and returns the maximum version found in a comment. if no matching comment is found,
// the function returns version 0.0.0
func getMaxVersionComment(config *model.Config, maintainer *model.Maintainer, issue model.Issue, comments []*model.Comment) *version.Version {
approverm := map[string]bool{}
approvers := []*model.Person{}

maxVersion, _ := version.NewVersion("0.0.0")

matcher, err := regexp.Compile(config.Pattern)
if err != nil {
// this should never happen
return maxVersion
}

for _, comment := range comments {
// cannot lgtm your own pull request
if config.SelfApprovalOff && comment.Author == issue.Author {
continue
}
// the user must be a valid maintainer of the project
person, ok := maintainer.People[comment.Author]
if !ok {
continue
}
// the same author can't approve something twice
if _, ok := approverm[comment.Author]; ok {
continue
}
// verify the comment matches the approval pattern
m := matcher.FindStringSubmatch(comment.Body)
if len(m) > 0 {
approverm[comment.Author] = true
approvers = append(approvers, person)

if len(m) > 1 {
//has a version
curVersion, err := version.NewVersion(m[1])
if err != nil {
continue
}
if curVersion.GreaterThan(maxVersion) {
maxVersion = curVersion
}
}
}
}

return maxVersion
}

// getMaxExistingTag is a helper function that scans all passed-in tags for a
// comments with semantic versions. It returns the max version found. If no version
// is found, the function returns a version with the value 0.0.0
Expand All @@ -220,3 +171,34 @@ func getMaxExistingTag(tags []model.Tag) *version.Version {
log.Debugf("maxVer found is %s", maxVer.String())
return maxVer
}

// getMaxVersionComment is a helper function that analyzes the list of comments
// and returns the maximum version found in a comment. if no matching comment is found,
// 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 {
maxVersion, _ := version.NewVersion("0.0.0")
ma, err := regexp.Compile(config.Pattern)
if err != nil {
//this should never happen, unless a bad pattern was provided by the user
log.Errorf("Invalid version pattern provided so no comment version tagging will be done: %s", config.Pattern)
return nil
}
matcher(config, maintainer, &issue, comments, func(maintainer *model.Maintainer, comment *model.Comment) {
// verify the comment matches the approval pattern
m := ma.FindStringSubmatch(comment.Body)
if len(m) > 1 {
//has a version
curVersion, err := version.NewVersion(m[1])
if err != nil {
return
}
if maxVersion == nil || curVersion.GreaterThan(maxVersion) {
maxVersion = curVersion
}
}
})

return maxVersion
}
Loading

0 comments on commit aa3adad

Please sign in to comment.