diff --git a/checks/code_review_test.go b/checks/code_review_test.go index 340b74aebe65..5080f7c24ab0 100644 --- a/checks/code_review_test.go +++ b/checks/code_review_test.go @@ -187,6 +187,51 @@ func TestCodereview(t *testing.T) { Score: 5, }, }, + { + name: "Valid Phabricator commit", + commits: []clients.Commit{ + { + SHA: "sha", + Committer: clients.User{ + Login: "bob", + }, + Message: "Title\nReviewed By: alice\nDifferential Revision: PHAB234", + }, + }, + expected: checker.CheckResult{ + Score: 10, + }, + }, + { + name: "Phabricator like, missing differential", + commits: []clients.Commit{ + { + SHA: "sha", + Committer: clients.User{ + Login: "bob", + }, + Message: "Title\nReviewed By: alice", + }, + }, + expected: checker.CheckResult{ + Score: 0, + }, + }, + { + name: "Phabricator like, missing reviewed by", + commits: []clients.Commit{ + { + SHA: "sha", + Committer: clients.User{ + Login: "bob", + }, + Message: "Title\nDifferential Revision: PHAB234", + }, + }, + expected: checker.CheckResult{ + Score: 0, + }, + }, } for _, tt := range tests { diff --git a/checks/evaluation/code_review.go b/checks/evaluation/code_review.go index cf46d603af11..7b72de44fb69 100644 --- a/checks/evaluation/code_review.go +++ b/checks/evaluation/code_review.go @@ -23,9 +23,10 @@ import ( ) var ( - reviewPlatformGitHub = "GitHub" - reviewPlatformProw = "Prow" - reviewPlatformGerrit = "Gerrit" + reviewPlatformGitHub = "GitHub" + reviewPlatformProw = "Prow" + reviewPlatformGerrit = "Gerrit" + reviewPlatformPhabricator = "Phabricator" ) // CodeReview applies the score policy for the Code-Review check. @@ -42,10 +43,11 @@ func CodeReview(name string, dl checker.DetailLogger, } totalReviewed := map[string]int{ - // The 3 platforms we support. - reviewPlatformGitHub: 0, - reviewPlatformProw: 0, - reviewPlatformGerrit: 0, + // The 4 platforms we support. + reviewPlatformGitHub: 0, + reviewPlatformProw: 0, + reviewPlatformGerrit: 0, + reviewPlatformPhabricator: 0, } for i := range r.DefaultBranchCommits { @@ -64,7 +66,8 @@ func CodeReview(name string, dl checker.DetailLogger, if totalReviewed[reviewPlatformGitHub] == 0 && totalReviewed[reviewPlatformGerrit] == 0 && - totalReviewed[reviewPlatformProw] == 0 { + totalReviewed[reviewPlatformProw] == 0 && + totalReviewed[reviewPlatformPhabricator] == 0 { return checker.CreateMinScoreResult(name, "no reviews found") } @@ -111,6 +114,9 @@ func getApprovedReviewSystem(c *checker.DefaultBranchCommit, dl checker.DetailLo case isReviewedOnGerrit(c, dl): return reviewPlatformGerrit + + case isReviewedOnPhabricator(c, dl): + return reviewPlatformPhabricator } return "" @@ -187,3 +193,22 @@ func isReviewedOnGerrit(c *checker.DefaultBranchCommit, dl checker.DetailLogger) } return false } + +func isReviewedOnPhabricator(c *checker.DefaultBranchCommit, dl checker.DetailLogger) bool { + if isBot(c.Committer.Login) { + dl.Debug(&checker.LogMessage{ + Text: fmt.Sprintf("skip commit %s from bot account: %s", c.SHA, c.Committer.Login), + }) + return true + } + + m := c.CommitMessage + if strings.Contains(m, "\nDifferential Revision: ") && + strings.Contains(m, "\nReviewed By: ") { + dl.Debug(&checker.LogMessage{ + Text: fmt.Sprintf("commit %s was approved through %s", c.SHA, reviewPlatformPhabricator), + }) + return true + } + return false +} diff --git a/go.mod b/go.mod index 4f13dd642cd3..488bd52ae5a6 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( require ( cloud.google.com/go/bigquery v1.31.0 cloud.google.com/go/monitoring v1.4.0 // indirect - cloud.google.com/go/pubsub v1.21.0 + cloud.google.com/go/pubsub v1.21.1 cloud.google.com/go/trace v1.2.0 // indirect contrib.go.opencensus.io/exporter/stackdriver v0.13.12 github.com/bombsimon/logrusr/v2 v2.0.1 @@ -37,7 +37,7 @@ require ( gocloud.dev v0.25.0 golang.org/x/text v0.3.7 golang.org/x/tools v0.1.10 - google.golang.org/genproto v0.0.0-20220407144326-9054f6ed7bac + google.golang.org/genproto v0.0.0-20220426171045-31bebdecfb46 google.golang.org/protobuf v1.28.0 gopkg.in/yaml.v2 v2.4.0 gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b @@ -53,7 +53,7 @@ require ( require ( cloud.google.com/go v0.100.2 // indirect - cloud.google.com/go/compute v1.5.0 // indirect + cloud.google.com/go/compute v1.6.0 // indirect cloud.google.com/go/iam v0.3.0 // indirect cloud.google.com/go/storage v1.22.0 // indirect github.com/Microsoft/go-winio v0.5.1 // indirect @@ -104,12 +104,12 @@ require ( github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect golang.org/x/crypto v0.0.0-20220331220935-ae2d96664a29 // indirect - golang.org/x/net v0.0.0-20220401154927-543a649e0bdd // indirect + golang.org/x/net v0.0.0-20220412020605-290c469a71a5 // indirect golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5 // indirect golang.org/x/sync v0.0.0-20210220032951-036812b2e83c // indirect - golang.org/x/sys v0.0.0-20220330033206-e17cdc41300f // indirect + golang.org/x/sys v0.0.0-20220412211240-33da011f77ad // indirect golang.org/x/xerrors v0.0.0-20220411194840-2f41105eb62f // indirect - google.golang.org/api v0.74.0 // indirect + google.golang.org/api v0.76.0 // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/grpc v1.45.0 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect diff --git a/go.sum b/go.sum index 5079dddea76d..a7b53c97d83e 100644 --- a/go.sum +++ b/go.sum @@ -50,8 +50,9 @@ cloud.google.com/go/bigquery v1.31.0/go.mod h1:jcC2eG41XaQcuaG9/e7AseL/AxVO3RAxS cloud.google.com/go/compute v0.1.0/go.mod h1:GAesmwr110a34z04OlxYkATPBEfVhkymfTBXtfbBFow= cloud.google.com/go/compute v1.2.0/go.mod h1:xlogom/6gr8RJGBe7nT2eGsQYAFUbbv8dbC29qE3Xmw= cloud.google.com/go/compute v1.3.0/go.mod h1:cCZiE1NHEtai4wiufUhW8I8S1JKkAnhnQJWM7YD99wM= -cloud.google.com/go/compute v1.5.0 h1:b1zWmYuuHz7gO9kDcM/EpHGr06UgsYNRpNJzI2kFiLM= cloud.google.com/go/compute v1.5.0/go.mod h1:9SMHyhJlzhlkJqrPAc839t2BZFTSk6Jdj6mkzQJeu0M= +cloud.google.com/go/compute v1.6.0 h1:XdQIN5mdPTSBVwSIVDuY5e8ZzVAccsHvD3qTEz4zIps= +cloud.google.com/go/compute v1.6.0/go.mod h1:T29tfhtVbq1wvAPo0E3+7vhgmkOYeXjhFvz/FMzPu0s= cloud.google.com/go/datacatalog v1.3.0 h1:3llKXv7cC1acsWjvWmG0NQQkYVSVgunMSfVk7h6zz8Q= cloud.google.com/go/datacatalog v1.3.0/go.mod h1:g9svFY6tuR+j+hrTw3J2dNcmI0dzmSiyOzm8kpLq0a0= cloud.google.com/go/datastore v1.0.0/go.mod h1:LXYbyblFSglQ5pkeyhO+Qmw7ukd3C+pD7TKLgZqpHYE= @@ -72,8 +73,8 @@ cloud.google.com/go/pubsub v1.1.0/go.mod h1:EwwdRX2sKPjnvnqCa270oGRyludottCI76h+ cloud.google.com/go/pubsub v1.2.0/go.mod h1:jhfEVHT8odbXTkndysNHCcx0awwzvfOlguIAii9o8iA= cloud.google.com/go/pubsub v1.3.1/go.mod h1:i+ucay31+CNRpDW4Lu78I4xXG+O1r/MAHgjpRVR+TSU= cloud.google.com/go/pubsub v1.19.0/go.mod h1:/O9kmSe9bb9KRnIAWkzmqhPjHo6LtzGOBYd/kr06XSs= -cloud.google.com/go/pubsub v1.21.0 h1:mWyK5FZR1B1cwRUGVk2i+s4PMEQTivsO7V9AkzWEL7w= -cloud.google.com/go/pubsub v1.21.0/go.mod h1:W8O6eOJ153eZew+q6ZT7bj8tUEIVudi29UlXXzWGgT4= +cloud.google.com/go/pubsub v1.21.1 h1:ghu6wlm6WouITmmuwkxGG+6vNRXDaPdAjqLcRdsw3EQ= +cloud.google.com/go/pubsub v1.21.1/go.mod h1:u3XGeMBOBCIQLcxNzy14Svz88ZFS8vI250uDgIAQDSQ= cloud.google.com/go/secretmanager v1.3.0/go.mod h1:+oLTkouyiYiabAQNugCeTS3PAArGiMJuBqvJnJsyH+U= cloud.google.com/go/storage v1.0.0/go.mod h1:IhtSnM/ZTZV8YYJWCY8RULGVqBDmpoyjwiyrjsg+URw= cloud.google.com/go/storage v1.5.0/go.mod h1:tpKbwo567HUNpVclU5sGELwQWBDZ8gh0ZeosJ0Rtdos= @@ -1660,8 +1661,9 @@ golang.org/x/net v0.0.0-20220107192237-5cfca573fb4d/go.mod h1:9nx3DQGgdP8bBQD5qx golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/net v0.0.0-20220325170049-de3da57026de/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= -golang.org/x/net v0.0.0-20220401154927-543a649e0bdd h1:zYlwaUHTmxuf6H7hwO2dgwqozQmH7zf4x+/qql4oVWc= golang.org/x/net v0.0.0-20220401154927-543a649e0bdd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= +golang.org/x/net v0.0.0-20220412020605-290c469a71a5 h1:bRb386wvrE+oBNdF1d/Xh9mQrfQ4ecYhW5qJ5GvTGT4= +golang.org/x/net v0.0.0-20220412020605-290c469a71a5/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/oauth2 v0.0.0-20180724155351-3d292e4d0cdc/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181017192945-9dcd33a902f4/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= @@ -1832,8 +1834,9 @@ golang.org/x/sys v0.0.0-20220227234510-4e6760a101f9/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220315194320-039c03cc5b86/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220319134239-a9b59b0215f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220328115105-d36c6a25d886/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220330033206-e17cdc41300f h1:rlezHXNlxYWvBCzNses9Dlc7nGFaNMJeqLolcmQSSZY= golang.org/x/sys v0.0.0-20220330033206-e17cdc41300f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220412211240-33da011f77ad h1:ntjMns5wyP/fN65tdBD4g8J5w8n015+iIIs9rtjXkY0= +golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210916214954-140adaaadfaf/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= @@ -2017,8 +2020,9 @@ google.golang.org/api v0.68.0/go.mod h1:sOM8pTpwgflXRhz+oC8H2Dr+UcbMqkPPWNJo88Q7 google.golang.org/api v0.69.0/go.mod h1:boanBiw+h5c3s+tBPgEzLDRHfFLWV0qXxRHz3ws7C80= google.golang.org/api v0.70.0/go.mod h1:Bs4ZM2HGifEvXwd50TtW70ovgJffJYw2oRCOFU/SkfA= google.golang.org/api v0.71.0/go.mod h1:4PyU6e6JogV1f9eA4voyrTY2batOLdgZ5qZ5HOCc4j8= -google.golang.org/api v0.74.0 h1:ExR2D+5TYIrMphWgs5JCgwRhEDlPDXXrLwHHMgPHTXE= google.golang.org/api v0.74.0/go.mod h1:ZpfMZOVRMywNyvJFeqL9HRWBgAuRfSjJFpe9QtRRyDs= +google.golang.org/api v0.76.0 h1:UkZl25bR1FHNqtK/EKs3vCdpZtUO6gea3YElTwc8pQg= +google.golang.org/api v0.76.0/go.mod h1:pU9QmyHLnzlpar1Mjt4IbapUCy8J+6HD6GeELN69ljA= google.golang.org/appengine v1.1.0/go.mod h1:EbEs0AVv82hx2wNQdGPgUI5lhzA/G0D9YwlJXL52JkM= google.golang.org/appengine v1.2.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/appengine v1.3.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= @@ -2125,8 +2129,11 @@ google.golang.org/genproto v0.0.0-20220310185008-1973136f34c6/go.mod h1:kGP+zUP2 google.golang.org/genproto v0.0.0-20220324131243-acbaeb5b85eb/go.mod h1:hAL49I2IFola2sVEjAn7MEwsja0xp51I0tlGAf9hz4E= google.golang.org/genproto v0.0.0-20220401170504-314d38edb7de/go.mod h1:8w6bsBMX6yCPbAVTeqQHvzxW0EIFigd5lZyahWgyfDo= google.golang.org/genproto v0.0.0-20220405205423-9d709892a2bf/go.mod h1:8w6bsBMX6yCPbAVTeqQHvzxW0EIFigd5lZyahWgyfDo= -google.golang.org/genproto v0.0.0-20220407144326-9054f6ed7bac h1:qSNTkEN+L2mvWcLgJOR+8bdHX9rN/IdU3A1Ghpfb1Rg= google.golang.org/genproto v0.0.0-20220407144326-9054f6ed7bac/go.mod h1:8w6bsBMX6yCPbAVTeqQHvzxW0EIFigd5lZyahWgyfDo= +google.golang.org/genproto v0.0.0-20220413183235-5e96e2839df9/go.mod h1:8w6bsBMX6yCPbAVTeqQHvzxW0EIFigd5lZyahWgyfDo= +google.golang.org/genproto v0.0.0-20220414192740-2d67ff6cf2b4/go.mod h1:8w6bsBMX6yCPbAVTeqQHvzxW0EIFigd5lZyahWgyfDo= +google.golang.org/genproto v0.0.0-20220426171045-31bebdecfb46 h1:G1IeWbjrqEq9ChWxEuRPJu6laA67+XgTFHVSAvepr38= +google.golang.org/genproto v0.0.0-20220426171045-31bebdecfb46/go.mod h1:8w6bsBMX6yCPbAVTeqQHvzxW0EIFigd5lZyahWgyfDo= google.golang.org/grpc v1.14.0/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw= google.golang.org/grpc v1.16.0/go.mod h1:0JHn/cJsOMiMfNA9+DeHDlAU7KAAB5GDlYFpa9MZMio= google.golang.org/grpc v1.17.0/go.mod h1:6QZJwpn2B+Zp71q/5VxRsJ6NXXVCE5NRUHRo+f3cWCs=