mirror of
https://github.com/ossf/scorecard.git
synced 2024-09-17 11:57:12 +03:00
✨ Add support for Phabricator as a code review system (#1884)
* ✨ Add support for Phabricator as a code review system
Signed-off-by: Vihang Mehta <vihang@pixielabs.ai>
* Also look for Differential Revision: to ensure that this repo uses Phabricator
Signed-off-by: Vihang Mehta <vihang@pixielabs.ai>
* Add some unit tests to cover Phabricator Review detection
Signed-off-by: Vihang Mehta <vihang@pixielabs.ai>
This commit is contained in:
parent
f779fb8761
commit
72086c9d4c
@ -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 {
|
||||
|
@ -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
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user