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

Jump to bottom

Include required review count #211

Closed
wants to merge 9 commits into from
Closed

Include required review count #211

wants to merge 9 commits into from

Conversation

bltavares
Copy link
Contributor

This commit includes the required_approving_review_count field on the
github_branch_protection resource, allowing to manage the miminum
amount of reviewers to allow a merge to happen.

This field must be between 1 - 6, according to the docs, and must be
valid if present.

Bumping the go-github to v24 made it default to 0 when not
present, causing the followin error

GitHub API error when count is 0 
422 Invalid request.

No subschema in "anyOf" matched.
0 must be greater than or equal to 1.
Not all subschemas of "allOf" matched.
For 'anyOf/1', {"dismissal_restrictions"=>{"users"=>[], "teams"=>[]}, "dismiss_stale_reviews"=>false, "require_code_owner_reviews"=>true, "required_approving_review_count"=>0} is not a null. []

Payload:
{
 "required_status_checks": {
  "strict": true,
  "contexts": [
   "lint",
   "test"
  ]
 },
 "required_pull_request_reviews": {
  "dismissal_restrictions": {
   "users": [],
   "teams": []
  },
  "dismiss_stale_reviews": false,
  "require_code_owner_reviews": true,
  "required_approving_review_count": 0
 },
 "enforce_admins": true,
 "restrictions": null
}

This PR is important when upgrading go-github.

Built on top of: #207

mattburgess and others added 9 commits April 23, 2019 10:29
@mattburgess @bltavares
vendor: github.com/google/go-github/github@v17.0.0
9bee1d8 
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
@mattburgess @bltavares
vendor: github.com/google/go-github/github@v18.0.0
1902385 
Update imports as go-github converted to using modules starting with
v18.

Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
@mattburgess @bltavares
vendor: github.com/google/go-github/github@v18.1.0
58d0a91 
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
@mattburgess @bltavares
vendor: github.com/google/go-github/github@v18.2.0
feba2ae 
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
@mattburgess @bltavares
vendor: github.com/google/go-github/github@v19.0.0
f8097bd 
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
@mattburgess @bltavares
vendor: github.com/google/go-github/github@v20.0.0
f385348 
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
@mattburgess @bltavares
vendor: github.com/google/go-github/github@v21.0.0
66f6f4f 
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
@mattburgess @bltavares
vendor: github.com/google/go-github/github@v24.0.0
19c8a57 
Signed-off-by: Trevor Bramwell <tbramwell@linuxfoundation.org>
@bltavares
Include required review count
2d01f85 
This commit includes the `required_approving_review_count` field on the
`github_branch_protection` resource, allowing to manage the miminum
amount of reviewers to allow a merge to happen.

This field must be between 1 - 6, according to the docs, and must be
valid if present.

Bumping the `go-github` to `v24` made it default to `0` when not
present, causing the followin error

<detail>
<summary>GitHub API error when count is 0</summary>

```
422 Invalid request.

No subschema in "anyOf" matched.
0 must be greater than or equal to 1.
Not all subschemas of "allOf" matched.
For 'anyOf/1', {"dismissal_restrictions"=>{"users"=>[], "teams"=>[]}, "dismiss_stale_reviews"=>false, "require_code_owner_reviews"=>true, "required_approving_review_count"=>0} is not a null. []

Payload:
{
 "required_status_checks": {
  "strict": true,
  "contexts": [
   "lint",
   "test"
  ]
 },
 "required_pull_request_reviews": {
  "dismissal_restrictions": {
   "users": [],
   "teams": []
  },
  "dismiss_stale_reviews": false,
  "require_code_owner_reviews": true,
  "required_approving_review_count": 0
 },
 "enforce_admins": true,
 "restrictions": null
}
```
</detail>

This PR is important when upgrading `go-github`.

Built on top of: https://github.com/terraform-providers/terraform-provider-github/pull/207
@ghost ghost added the size/XXL label Apr 23, 2019
bltavares referenced this pull request in bltavares/terraform-provider-github Apr 23, 2019
@bltavares
Provide "require signed commits" on github_branch_protection
8ee9906 
This commit provides changing the toggle to require signed commits on
branch protection.

The [API for signed commits](https://developer.github.com/v3/repos/branches/#get-required-signatures-of-protected-branch)
is separate endpoint from the branch protection url, and does not come on the same payload.
This means that we need to make an extra request to read and modify it.

This changes requires `go-github` at least v19.

Need:
 - [ ] Update `go-github` version: https://github.com/terraform-providers/terraform-provider-github/pull/207
 - [ ] Provide default required reviewers count to avoid GitHub update error: https://github.com/terraform-providers/terraform-provider-github/pull/211
Closes https://github.com/terraform-providers/terraform-provider-github/issues/87
@bltavares bltavares mentioned this pull request Apr 23, 2019
Closed
2 tasks
@bltavares
Copy link
Contributor Author

I'll gladly rebase or pick the last commit apart if necessary, but updating the version of go-github is important for it. After #207 lands, I'll gladly update the PR.

Note: precompiled plugin to validate the changes here

@bltavares bltavares mentioned this pull request Apr 23, 2019
Closed
@bltavares
Copy link
Contributor Author

bltavares commented Apr 24, 2019 •
edited
Loading

@apparentlymart sorry to bother here.

This change is important to include on sdk-v0.12-beta2 as bumping go-github will send a default value that GitHub's API don't allow. When the official release comes, sending the default value as 0 instead of 1 will make any update by this provider fail.

I've opened a PR (https://github.com/terraform-providers/terraform-provider-github/pull/213) against sdk-v0.12-beta2

bltavares referenced this pull request in bltavares/terraform-provider-github Apr 24, 2019
@bltavares
Provide "require signed commits" on github_branch_protection
fb80993 
This commit provides changing the toggle to require signed commits on
branch protection.

The [API for signed commits](https://developer.github.com/v3/repos/branches/#get-required-signatures-of-protected-branch)
is separate endpoint from the branch protection url, and does not come on the same payload.
This means that we need to make an extra request to read and modify it.

This changes requires `go-github` at least v19.

Need:
 - [ ] Update `go-github` version: https://github.com/terraform-providers/terraform-provider-github/pull/207
 - [ ] Provide default required reviewers count to avoid GitHub update error: https://github.com/terraform-providers/terraform-provider-github/pull/211
Closes https://github.com/terraform-providers/terraform-provider-github/issues/87
bltavares referenced this pull request in bltavares/terraform-provider-github Apr 24, 2019
@bltavares
Provide "require signed commits" on github_branch_protection
a93cf11 
This commit provides changing the toggle to require signed commits on
branch protection.

The [API for signed commits](https://developer.github.com/v3/repos/branches/#get-required-signatures-of-protected-branch)
is separate endpoint from the branch protection url, and does not come on the same payload.
This means that we need to make an extra request to read and modify it.

This changes requires `go-github` at least v19.

Need:
 - [ ] Update `go-github` version: https://github.com/terraform-providers/terraform-provider-github/pull/207
 - [ ] Provide default required reviewers count to avoid GitHub update error: https://github.com/terraform-providers/terraform-provider-github/pull/211
Closes https://github.com/terraform-providers/terraform-provider-github/issues/87
@paultyng paultyng mentioned this pull request May 2, 2019
Merged
@paultyng paultyng closed this May 2, 2019
bltavares referenced this pull request in bltavares/terraform-provider-github May 2, 2019
@bltavares
Provide "require signed commits" on github_branch_protection
eaa247d 
This commit provides changing the toggle to require signed commits on
branch protection.

The [API for signed commits](https://developer.github.com/v3/repos/branches/#get-required-signatures-of-protected-branch)
is separate endpoint from the branch protection url, and does not come on the same payload.
This means that we need to make an extra request to read and modify it.

This changes requires `go-github` at least v19.

Need:
 - [ ] Update `go-github` version: https://github.com/terraform-providers/terraform-provider-github/pull/207
 - [ ] Provide default required reviewers count to avoid GitHub update error: https://github.com/terraform-providers/terraform-provider-github/pull/211
Closes https://github.com/terraform-providers/terraform-provider-github/issues/87
@paultyng
Copy link
Contributor

paultyng commented May 2, 2019

2.0.0 has been released, including support for this attribute.

tracypholmes referenced this pull request Jun 27, 2019
@bltavares @tracypholmes
Provide "require signed commits" on github_branch_protection (#214)
832ea7e 
* Provide "require signed commits" on `github_branch_protection`

This commit provides changing the toggle to require signed commits on
branch protection.

The [API for signed commits](https://developer.github.com/v3/repos/branches/#get-required-signatures-of-protected-branch)
is separate endpoint from the branch protection url, and does not come on the same payload.
This means that we need to make an extra request to read and modify it.

This changes requires `go-github` at least v19.

Need:
 - [ ] Update `go-github` version: https://github.com/terraform-providers/terraform-provider-github/pull/207
 - [ ] Provide default required reviewers count to avoid GitHub update error: https://github.com/terraform-providers/terraform-provider-github/pull/211
Closes https://github.com/terraform-providers/terraform-provider-github/issues/87

* Include tests

* Adds documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@comigor comigor comigor approved these changes

Assignees
No one assigned
Labels
size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bltavares @paultyng @comigor @mattburgess