Repository: google/go-github
Pull request #1207 (2019-06-24 11:40:36)

Consistent errors for GetRef when a Ref doesn't exist

pilbot: Fixes #1208 
gmlewis: This is fine with me. What do you think, @gauntface ?

If we do this, I believe it would fall under the category of a "minor" semver change... meaning that we would bump the middle number. It doesn't break any existing clients because it still returns an error, but the error response does change.

If @gauntface is fine with this change, then LGTM.

Patch:
diff --git a/AUTHORS b/AUTHORS
index afb59217..b1faeb0f 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -27,6 +27,7 @@ Andrew Ryabchun <aryabchun@mail.ua>
 Andy Grunwald <andygrunwald@gmail.com>
 Andy Hume <andyhume@gmail.com>
 Andy Lindeman <andy@lindeman.io>
+anjanashenoy <anjanashenoy1@gmail.com>
 Anshuman Bhartiya <anshuman.bhartiya@gmail.com>
 Antoine <antoine.tu@mail.mcgill.ca>
 Antoine Pelisse <apelisse@gmail.com>
@@ -152,6 +153,7 @@ Luke Kysow <lkysow@gmail.com>
 Luke Roberts <email@luke-roberts.co.uk>
 Luke Young <luke@hydrantlabs.org>
 Maksim Zhylinski <uzzable@gmail.com>
+Mark Tareshawty <tarebyte@github.com>
 Martin-Louis Bright <mlbright@gmail.com>
 Marwan Sulaiman <marwan.sameer@gmail.com>
 Mat Geist <matgeist@gmail.com>
diff --git a/github/git_refs.go b/github/git_refs.go
index 3f381d5f..fe16880e 100644
--- a/github/git_refs.go
+++ b/github/git_refs.go
@@ -68,7 +68,10 @@ func (s *GitService) GetRef(ctx context.Context, owner string, repo string, ref
 	resp, err := s.client.Do(ctx, req, r)
 	if _, ok := err.(*json.UnmarshalTypeError); ok {
 		// Multiple refs, means there wasn't an exact match.
-		return nil, resp, errors.New("no exact match found for this ref")
+		return nil, resp, errors.New("multiple matches found for this ref")
+	} else if resp.StatusCode == 404 {
+		// No ref, there was no match for the ref
+		return nil, resp, errors.New("no match found for this ref")
 	} else if err != nil {
 		return nil, resp, err
 	}
diff --git a/github/github-accessors.go b/github/github-accessors.go
index 0bb206bd..9218107b 100644
--- a/github/github-accessors.go
+++ b/github/github-accessors.go
@@ -9940,6 +9940,14 @@ func (r *RepositoryComment) GetID() int64 {
 	return *r.ID
 }
 
+// GetNodeID returns the NodeID field if it's non-nil, zero value otherwise.
+func (r *RepositoryComment) GetNodeID() string {
+	if r == nil || r.NodeID == nil {
+		return ""
+	}
+	return *r.NodeID
+}
+
 // GetPath returns the Path field if it's non-nil, zero value otherwise.
 func (r *RepositoryComment) GetPath() string {
 	if r == nil || r.Path == nil {
@@ -12156,6 +12164,14 @@ func (u *User) GetID() int64 {
 	return *u.ID
 }
 
+// GetLdapDn returns the LdapDn field if it's non-nil, zero value otherwise.
+func (u *User) GetLdapDn() string {
+	if u == nil || u.LdapDn == nil {
+		return ""
+	}
+	return *u.LdapDn
+}
+
 // GetLocation returns the Location field if it's non-nil, zero value otherwise.
 func (u *User) GetLocation() string {
 	if u == nil || u.Location == nil {
@@ -12372,6 +12388,14 @@ func (u *UserEmail) GetVerified() bool {
 	return *u.Verified
 }
 
+// GetVisibility returns the Visibility field if it's non-nil, zero value otherwise.
+func (u *UserEmail) GetVisibility() string {
+	if u == nil || u.Visibility == nil {
+		return ""
+	}
+	return *u.Visibility
+}
+
 // GetAvatarURL returns the AvatarURL field if it's non-nil, zero value otherwise.
 func (u *UserLDAPMapping) GetAvatarURL() string {
 	if u == nil || u.AvatarURL == nil {
diff --git a/github/repos_comments.go b/github/repos_comments.go
index fa2377d4..380ca3d3 100644
--- a/github/repos_comments.go
+++ b/github/repos_comments.go
@@ -16,6 +16,7 @@ type RepositoryComment struct {
 	HTMLURL   *string    `json:"html_url,omitempty"`
 	URL       *string    `json:"url,omitempty"`
 	ID        *int64     `json:"id,omitempty"`
+	NodeID    *string    `json:"node_id,omitempty"`
 	CommitID  *string    `json:"commit_id,omitempty"`
 	User      *User      `json:"user,omitempty"`
 	Reactions *Reactions `json:"reactions,omitempty"`
diff --git a/github/users.go b/github/users.go
index 87cfa7f8..2592aea0 100644
--- a/github/users.go
+++ b/github/users.go
@@ -47,6 +47,7 @@ type User struct {
 	Collaborators           *int       `json:"collaborators,omitempty"`
 	TwoFactorAuthentication *bool      `json:"two_factor_authentication,omitempty"`
 	Plan                    *Plan      `json:"plan,omitempty"`
+	LdapDn                  *string    `json:"ldap_dn,omitempty"`
 
 	// API URLs
 	URL               *string `json:"url,omitempty"`
diff --git a/github/users_emails.go b/github/users_emails.go
index 0bbd4627..78d21491 100644
--- a/github/users_emails.go
+++ b/github/users_emails.go
@@ -9,9 +9,10 @@ import "context"
 
 // UserEmail represents user's email address
 type UserEmail struct {
-	Email    *string `json:"email,omitempty"`
-	Primary  *bool   `json:"primary,omitempty"`
-	Verified *bool   `json:"verified,omitempty"`
+	Email      *string `json:"email,omitempty"`
+	Primary    *bool   `json:"primary,omitempty"`
+	Verified   *bool   `json:"verified,omitempty"`
+	Visibility *string `json:"visibility,omitempty"`
 }
 
 // ListEmails lists all email addresses for the authenticated user.


Test patch:
diff --git a/github/git_refs_test.go b/github/git_refs_test.go
index 3cae3c08..6c8a1d87 100644
--- a/github/git_refs_test.go
+++ b/github/git_refs_test.go
@@ -56,6 +56,23 @@ func TestGitService_GetRef_singleRef(t *testing.T) {
 	}
 }
 
+func TestGitService_GetRef_noRefs(t *testing.T) {
+	client, mux, _, teardown := setup()
+	defer teardown()
+
+	mux.HandleFunc("/repos/o/r/git/refs/heads/b", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		w.WriteHeader(http.StatusNotFound)
+		fmt.Fprint(w, "[]")
+	})
+
+	_, _, err := client.Git.GetRef(context.Background(), "o", "r", "refs/heads/b")
+	want := "no match found for this ref"
+	if err.Error() != want {
+		t.Errorf("Git.GetRef returned %+v, want %+v", err, want)
+	}
+}
+
 func TestGitService_GetRef_multipleRefs(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
@@ -87,7 +104,7 @@ func TestGitService_GetRef_multipleRefs(t *testing.T) {
 	})
 
 	_, _, err := client.Git.GetRef(context.Background(), "o", "r", "refs/heads/b")
-	want := "no exact match found for this ref"
+	want := "multiple matches found for this ref"
 	if err.Error() != want {
 		t.Errorf("Git.GetRef returned %+v, want %+v", err, want)
 	}
diff --git a/github/github-stringify_test.go b/github/github-stringify_test.go
index 2271a9d2..6934f237 100644
--- a/github/github-stringify_test.go
+++ b/github/github-stringify_test.go
@@ -1262,6 +1262,7 @@ func TestRepositoryComment_String(t *testing.T) {
 		HTMLURL:   String(""),
 		URL:       String(""),
 		ID:        Int64(0),
+		NodeID:    String(""),
 		CommitID:  String(""),
 		User:      &User{},
 		Reactions: &Reactions{},
@@ -1269,7 +1270,7 @@ func TestRepositoryComment_String(t *testing.T) {
 		Path:      String(""),
 		Position:  Int(0),
 	}
-	want := `github.RepositoryComment{HTMLURL:"", URL:"", ID:0, CommitID:"", User:github.User{}, Reactions:github.Reactions{}, Body:"", Path:"", Position:0}`
+	want := `github.RepositoryComment{HTMLURL:"", URL:"", ID:0, NodeID:"", CommitID:"", User:github.User{}, Reactions:github.Reactions{}, Body:"", Path:"", Position:0}`
 	if got := v.String(); got != want {
 		t.Errorf("RepositoryComment.String = %v, want %v", got, want)
 	}
@@ -1518,6 +1519,7 @@ func TestUser_String(t *testing.T) {
 		Collaborators:           Int(0),
 		TwoFactorAuthentication: Bool(false),
 		Plan:                    &Plan{},
+		LdapDn:                  String(""),
 		URL:                     String(""),
 		EventsURL:               String(""),
 		FollowingURL:            String(""),
@@ -1529,7 +1531,7 @@ func TestUser_String(t *testing.T) {
 		StarredURL:              String(""),
 		SubscriptionsURL:        String(""),
 	}
-	want := `github.User{Login:"", ID:0, NodeID:"", AvatarURL:"", HTMLURL:"", GravatarID:"", Name:"", Company:"", Blog:"", Location:"", Email:"", Hireable:false, Bio:"", PublicRepos:0, PublicGists:0, Followers:0, Following:0, CreatedAt:github.Timestamp{0001-01-01 00:00:00 +0000 UTC}, UpdatedAt:github.Timestamp{0001-01-01 00:00:00 +0000 UTC}, SuspendedAt:github.Timestamp{0001-01-01 00:00:00 +0000 UTC}, Type:"", SiteAdmin:false, TotalPrivateRepos:0, OwnedPrivateRepos:0, PrivateGists:0, DiskUsage:0, Collaborators:0, TwoFactorAuthentication:false, Plan:github.Plan{}, URL:"", EventsURL:"", FollowingURL:"", FollowersURL:"", GistsURL:"", OrganizationsURL:"", ReceivedEventsURL:"", ReposURL:"", StarredURL:"", SubscriptionsURL:""}`
+	want := `github.User{Login:"", ID:0, NodeID:"", AvatarURL:"", HTMLURL:"", GravatarID:"", Name:"", Company:"", Blog:"", Location:"", Email:"", Hireable:false, Bio:"", PublicRepos:0, PublicGists:0, Followers:0, Following:0, CreatedAt:github.Timestamp{0001-01-01 00:00:00 +0000 UTC}, UpdatedAt:github.Timestamp{0001-01-01 00:00:00 +0000 UTC}, SuspendedAt:github.Timestamp{0001-01-01 00:00:00 +0000 UTC}, Type:"", SiteAdmin:false, TotalPrivateRepos:0, OwnedPrivateRepos:0, PrivateGists:0, DiskUsage:0, Collaborators:0, TwoFactorAuthentication:false, Plan:github.Plan{}, LdapDn:"", URL:"", EventsURL:"", FollowingURL:"", FollowersURL:"", GistsURL:"", OrganizationsURL:"", ReceivedEventsURL:"", ReposURL:"", StarredURL:"", SubscriptionsURL:""}`
 	if got := v.String(); got != want {
 		t.Errorf("User.String = %v, want %v", got, want)
 	}