Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Required signatures on protected branches #1039

Merged
merged 9 commits into from
Nov 9, 2018
Merged

Required signatures on protected branches #1039

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
dfdbc92
Added support for github api preview signed commits.
dlnilsson Oct 29, 2018
c5c33fc
added SignaturesProtectedBranch getURL
dlnilsson Oct 29, 2018
00f96c6
Corrected grammar and naming convention.
dlnilsson Nov 2, 2018
302d642
Added pointer for Enabled bool.
dlnilsson Nov 2, 2018
ae38ea7
added github-accessor for GetEnabled()
dlnilsson Nov 2, 2018
d697b01
go fmt
dlnilsson Nov 2, 2018
b521515
re-added assertion of Enabled
dlnilsson Nov 7, 2018
9d0cbac
rename test func name to reflect refactor.
dlnilsson Nov 8, 2018
6b75719
keep Enabled json key if empty.
dlnilsson Nov 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions github/github-accessors.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion github/repos.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ type DismissalRestrictionsRequest struct {
type SignaturesProtectedBranch struct {
URL *string `json:"url,omitempty"`
// Commits pushed to matching branches must have verified signatures.
Enabled bool `json:"strict,omitempty"`
Enabled *bool `json:"strict,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/strict/enabled/

}

// ListBranches lists branches for the specified repository.
Expand Down
8 changes: 4 additions & 4 deletions github/repos_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1066,13 +1066,13 @@ func TestRepositoriesService_GetSignaturesProtectedBranch(t *testing.T) {
})

signature, _, err := client.Repositories.GetSignaturesProtectedBranch(context.Background(), "o", "r", "b")

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please delete this blank line, as we attempt to keep the error checking code tightly bound to the section of code that created it.

if err != nil {
t.Errorf("Repositories.GetSignaturesProtectedBranch returned error: %v", err)
}

want := &SignaturesProtectedBranch{
URL: String("/repos/o/r/branches/b/protection/required_signatures"),
Enabled: false,
URL: String("/repos/o/r/branches/b/protection/required_signatures"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add back your Enabled: Bool(false), line here and below.

}

if !reflect.DeepEqual(signature, want) {
Expand All @@ -1096,9 +1096,9 @@ func TestRepositoriesService_AddSignatureProtectedBranch(t *testing.T) {
}

want := &SignaturesProtectedBranch{
URL: String("/repos/o/r/branches/b/protection/required_signatures"),
Enabled: false,
URL: String("/repos/o/r/branches/b/protection/required_signatures"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add back your Enabled: Bool(false), line here.

}

if !reflect.DeepEqual(signature, want) {
t.Errorf("Repositories.RequireSignaturesOnProtectedBranch returned %+v, want %+v", signature, want)
}
Expand Down