Skip to content

Commit 4e6d076

Browse files
committed
Fix output so final status is last
1 parent 985c774 commit 4e6d076

File tree

3 files changed

+31
-26
lines changed

3 files changed

+31
-26
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ Code Ownership & Review Assignment Tool - GitHub CODEOWNERS but better
44

55
[![Go Report Card](https://goreportcard.com/badge/github.com/multimediallc/codeowners-plus)](https://goreportcard.com/report/github.com/multimediallc/codeowners-plus?kill_cache=1)
66
[![Tests](https://github.com/multimediallc/codeowners-plus/actions/workflows/go.yml/badge.svg)](https://github.com/multimediallc/codeowners-plus/actions/workflows/go.yml)
7-
![Coverage](https://img.shields.io/badge/Coverage-83.1%25-brightgreen)
7+
![Coverage](https://img.shields.io/badge/Coverage-83.0%25-brightgreen)
88
[![License](https://img.shields.io/badge/License-BSD%203--Clause-blue.svg)](https://opensource.org/licenses/BSD-3-Clause)
99
[![Contributor Covenant](https://img.shields.io/badge/Contributor%20Covenant-2.1-4baaaa.svg)](CODE_OF_CONDUCT.md)
1010

main.go

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -101,10 +101,10 @@ func NewApp(cfg AppConfig) (*App, error) {
101101
}
102102

103103
// Run executes the application logic
104-
func (a *App) Run() (bool, error) {
104+
func (a *App) Run() (bool, string, error) {
105105
// Initialize PR
106106
if err := a.client.InitPR(a.config.PR); err != nil {
107-
return false, fmt.Errorf("InitPR Error: %v", err)
107+
return false, "", fmt.Errorf("InitPR Error: %v", err)
108108
}
109109
printDebug("PR: %d\n", a.client.PR().GetNumber())
110110

@@ -127,14 +127,14 @@ func (a *App) Run() (bool, error) {
127127
printDebug("Getting diff for %s...%s\n", diffContext.Base, diffContext.Head)
128128
gitDiff, err := git.NewDiff(diffContext)
129129
if err != nil {
130-
return false, fmt.Errorf("NewGitDiff Error: %v", err)
130+
return false, "", fmt.Errorf("NewGitDiff Error: %v", err)
131131
}
132132
a.gitDiff = gitDiff
133133

134134
// Initialize codeowners
135135
codeOwners, err := codeowners.New(a.config.RepoDir, gitDiff.AllChanges(), WarningBuffer)
136136
if err != nil {
137-
return false, fmt.Errorf("NewCodeOwners Error: %v", err)
137+
return false, "", fmt.Errorf("NewCodeOwners Error: %v", err)
138138
}
139139
a.codeowners = codeOwners
140140

@@ -156,7 +156,8 @@ func (a *App) Run() (bool, error) {
156156
return a.processApprovalsAndReviewers()
157157
}
158158

159-
func (a *App) processApprovalsAndReviewers() (bool, error) {
159+
func (a *App) processApprovalsAndReviewers() (bool, string, error) {
160+
message := ""
160161
// Get all required owners before filtering
161162
allRequiredOwners := a.codeowners.AllRequired()
162163
allRequiredOwnerNames := allRequiredOwners.Flatten()
@@ -171,13 +172,13 @@ func (a *App) processApprovalsAndReviewers() (bool, error) {
171172

172173
// Initialize user reviewer map
173174
if err := a.client.InitUserReviewerMap(allRequiredOwnerNames); err != nil {
174-
return false, fmt.Errorf("InitUserReviewerMap Error: %v", err)
175+
return false, message, fmt.Errorf("InitUserReviewerMap Error: %v", err)
175176
}
176177

177178
// Get current approvals
178179
ghApprovals, err := a.client.GetCurrentReviewerApprovals()
179180
if err != nil {
180-
return false, fmt.Errorf("GetCurrentApprovals Error: %v", err)
181+
return false, message, fmt.Errorf("GetCurrentApprovals Error: %v", err)
181182
}
182183
printDebug("Current Approvals: %+v\n", ghApprovals)
183184

@@ -186,20 +187,20 @@ func (a *App) processApprovalsAndReviewers() (bool, error) {
186187
if a.conf.Enforcement.Approval {
187188
tokenOwnerApproval, err = a.processTokenOwnerApproval()
188189
if err != nil {
189-
return false, err
190+
return false, message, err
190191
}
191192
}
192193

193194
// Process approvals and dismiss stale ones
194195
validApprovalCount, err := a.processApprovals(ghApprovals)
195196
if err != nil {
196-
return false, err
197+
return false, message, err
197198
}
198199

199200
// Request reviews from required owners
200201
unapprovedOwners, err := a.requestReviews()
201202
if err != nil {
202-
return false, err
203+
return false, message, err
203204
}
204205

205206
maxReviewsMet := false
@@ -220,12 +221,12 @@ func (a *App) processApprovalsAndReviewers() (bool, error) {
220221
fiveDaysAgo := time.Now().AddDate(0, 0, -5)
221222
found, err := a.client.IsInComments(comment, &fiveDaysAgo)
222223
if err != nil {
223-
return false, fmt.Errorf("IsInComments Error: %v\n", err)
224+
return false, message, fmt.Errorf("IsInComments Error: %v\n", err)
224225
}
225226
if !found {
226227
err = a.client.AddComment(comment)
227228
if err != nil {
228-
return false, fmt.Errorf("AddComment Error: %v\n", err)
229+
return false, message, fmt.Errorf("AddComment Error: %v\n", err)
229230
}
230231
}
231232
}
@@ -240,13 +241,13 @@ func (a *App) processApprovalsAndReviewers() (bool, error) {
240241
return !found
241242
})
242243
if isInCommentsError != nil {
243-
return false, fmt.Errorf("IsInComments Error: %v\n", err)
244+
return false, message, fmt.Errorf("IsInComments Error: %v\n", err)
244245
}
245246
if len(viewersToPing) > 0 {
246247
comment := fmt.Sprintf("cc %s", strings.Join(viewersToPing, " "))
247248
err = a.client.AddComment(comment)
248249
if err != nil {
249-
return false, fmt.Errorf("AddComment Error: %v\n", err)
250+
return false, message, fmt.Errorf("AddComment Error: %v\n", err)
250251
}
251252
}
252253
}
@@ -260,29 +261,30 @@ func (a *App) processApprovalsAndReviewers() (bool, error) {
260261
if a.conf.Enforcement.Approval && tokenOwnerApproval != nil {
261262
_ = a.client.DismissStaleReviews([]*gh.CurrentApproval{tokenOwnerApproval})
262263
}
263-
printWarning(
264-
"FAIL: Codeowners reviews not satisfied\nStill required:\n - %s\n",
264+
message = fmt.Sprintf(
265+
"FAIL: Codeowners reviews not satisfied\nStill required:\n - %s",
265266
strings.Join(unapprovedCommentStrings, "\n - "),
266267
)
267-
return false, nil
268+
return false, message, nil
268269
}
269270

270271
// Exit if there are not enough reviews
271272
if a.conf.MinReviews != nil && *a.conf.MinReviews > 0 {
272273
if validApprovalCount < *a.conf.MinReviews {
273-
printWarning("FAIL: Min Reviews not satisfied. Need %d, found %d\n", *a.conf.MinReviews, validApprovalCount)
274-
return false, nil
274+
message = fmt.Sprintf("FAIL: Min Reviews not satisfied. Need %d, found %d", *a.conf.MinReviews, validApprovalCount)
275+
return false, message, nil
275276
}
276277
}
277278

279+
message = "Codeowners reviews satisfied"
278280
if a.conf.Enforcement.Approval && tokenOwnerApproval == nil {
279281
// Approve the PR since all codeowner teams have approved
280282
err = a.client.ApprovePR()
281283
if err != nil {
282-
return true, fmt.Errorf("ApprovePR Error: %v\n", err)
284+
return true, message, fmt.Errorf("ApprovePR Error: %v\n", err)
283285
}
284286
}
285-
return true, nil
287+
return true, message, nil
286288
}
287289

288290
func (a *App) processTokenOwnerApproval() (*gh.CurrentApproval, error) {
@@ -423,7 +425,7 @@ func main() {
423425
errorAndExit(true, "Failed to initialize app: %v\n", err)
424426
}
425427

426-
success, err := app.Run()
428+
success, message, err := app.Run()
427429
if err != nil {
428430
errorAndExit(true, "%v\n", err)
429431
}
@@ -438,8 +440,11 @@ func main() {
438440
}
439441
}
440442
if success {
441-
fmt.Println("Codeowners reviews satisfied")
442-
} else if app.conf.Enforcement.FailCheck {
443+
fmt.Fprintln(os.Stdout, message)
444+
} else {
445+
fmt.Fprintln(os.Stderr, message)
446+
}
447+
if !success && app.conf.Enforcement.FailCheck {
443448
os.Exit(1)
444449
}
445450
}

main_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -879,7 +879,7 @@ func TestProcessApprovalsAndReviewers(t *testing.T) {
879879
},
880880
}
881881

882-
success, err := app.processApprovalsAndReviewers()
882+
success, _, err := app.processApprovalsAndReviewers()
883883
if tc.expectError {
884884
if err == nil {
885885
t.Error("expected error but got none")

0 commit comments

Comments
 (0)