Handle protection status errors for unprotected branches by hemachandarv · Pull Request #2092 · google/go-github

hemachandarv · 2021-09-13T04:40:17Z

Resolves #625

hemachandarv · 2021-09-13T04:43:55Z

@gmlewis I've addressed #625 by returning an explicit error in the GET APIs. Do you think this is a good idea?

Also, do you think we should cover the same for the POST APIs too?

codecov · 2021-09-13T11:19:56Z

Codecov Report

Merging #2092 (0035635) into master (88b13f9) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2092      +/-   ##
==========================================
+ Coverage   97.75%   97.78%   +0.03%     
==========================================
  Files         107      109       +2     
  Lines        9601     9733     +132     
==========================================
+ Hits         9385     9517     +132     
  Misses        150      150              
  Partials       66       66

Impacted Files	Coverage Δ
github/repos.go	`98.66% <100.00%> (+0.01%)`	⬆️
github/event.go	`100.00% <0.00%> (ø)`
github/messages.go	`100.00% <0.00%> (ø)`
github/actions_runner_groups.go	`100.00% <0.00%> (ø)`
github/actions_workflow_runs.go	`100.00% <0.00%> (ø)`
github/orgs_hooks_deliveries.go	`100.00% <0.00%> (ø)`
github/repos_community_health.go	`100.00% <0.00%> (ø)`
github/repos_autolinks.go	`100.00% <0.00%> (ø)`
github/scim.go	`100.00% <0.00%> (ø)`
github/github.go	`97.58% <0.00%> (+<0.01%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88b13f9...0035635. Read the comment docs.

gmlewis

Thank you, @hemachandarv !
Just a few minor tweaks, please.

gmlewis · 2021-09-13T11:56:24Z

github/repos_test.go

+
+		w.WriteHeader(http.StatusBadRequest)
+		fmt.Fprintf(w, `{
+			"message": "Branch not protected",


Suggested change

"message": "Branch not protected",

"message": githubBranchNotProtected,

@gmlewis I'm not sure if we'll be able to use a variable inside a raw string. Am I missing something..?

Whups, you are correct. Please use %v and the variable name in that case.

Actually, %q makes more sense here.

Thanks @gmlewis. I've updated the PR with the suggested changes.

gmlewis · 2021-09-13T11:56:49Z

github/repos_test.go

+
+		w.WriteHeader(http.StatusBadRequest)
+		fmt.Fprintf(w, `{
+			"message": "Branch not protected",


Suggested change

"message": "Branch not protected",

"message": githubBranchNotProtected,

gmlewis · 2021-09-13T11:57:15Z

github/repos_test.go

+
+		w.WriteHeader(http.StatusBadRequest)
+		fmt.Fprintf(w, `{
+			"message": "Branch not protected",


Suggested change

"message": "Branch not protected",

"message": githubBranchNotProtected,

gmlewis · 2021-09-13T11:59:16Z

github/repos.go

+	if errorResponse, ok := err.(*ErrorResponse); ok {
+		return errorResponse.Message == githubBranchNotProtected
+	}
+	return false


Suggested change

if errorResponse, ok := err.(*ErrorResponse); ok {

return errorResponse.Message == githubBranchNotProtected

}

return false

errorResponse, ok := err.(*ErrorResponse)

return ok && errorResponse.Message == githubBranchNotProtected

@gmlewis Thank you for suggesting this change. I certainly missed this simplification.

gmlewis

Thank you, @hemachandarv !
LGTM.

Awaiting second LGTM before merging.

cpanato

LGTM
thanks!

gmlewis · 2021-12-03T14:47:12Z

Thank you, @cpanato !
Merging.

Handle protection status errors for unprotected branches

0daa71a

google-cla bot added the cla: yes Indication that the PR author has signed a Google Contributor License Agreement. label Sep 13, 2021

gmlewis requested changes Sep 13, 2021

View reviewed changes

gmlewis requested a review from wesleimp September 13, 2021 12:01

Address review comments

0035635

gmlewis approved these changes Sep 13, 2021

View reviewed changes

cpanato approved these changes Dec 3, 2021

View reviewed changes

gmlewis merged commit 85425ec into google:master Dec 3, 2021

gmlewis mentioned this pull request Jan 13, 2022

Bump version of go-github library to v42 #2253

Merged

bluekeyes mentioned this pull request Mar 22, 2022

Fix error evaluating unprotected branches palantir/bulldozer#302

Merged

	"message": "Branch not protected",
	"message": githubBranchNotProtected,

Conversation

hemachandarv commented Sep 13, 2021

Uh oh!

hemachandarv commented Sep 13, 2021

Uh oh!

codecov bot commented Sep 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmlewis left a comment

Choose a reason for hiding this comment

Uh oh!

cpanato left a comment

Choose a reason for hiding this comment

Uh oh!

gmlewis commented Dec 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Sep 13, 2021 •

edited

Loading