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.

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

Add checks field to RequiredStatusChecks #2276

Merged
merged 3 commits into from
Jan 31, 2022
Merged

Conversation

k24dizzle
Copy link
Contributor

@k24dizzle k24dizzle commented Jan 28, 2022

Attempt to solve #2274.

From my own experimentation, I discovered that there's two ways that this Checks field can be used to update the required status checks for a protected branch:

  • update branch protection -> via the ProtectionRequest -> RequiredStatusChecks -> RequiredStatusCheck struct, in this API call we must provide either one of Checks or Contexts (the other must be nil) in RequiredStatusChecks to successfully update a branch protection. If you provide both or neither, you will get an Invalid Request error.
  • update status check protection -> via the RequiredStatusChecksRequest -> RequiredStatusCheck struct, in this API call Checks and Contexts are both optional fields, if you provide both of them only Checks will be used to update the status checks.

@codecov
Copy link

codecov bot commented Jan 29, 2022

Codecov Report

Merging #2276 (d801426) into master (51df45c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2276   +/-   ##
=======================================
  Coverage   97.81%   97.81%           
=======================================
  Files         114      114           
  Lines       10266    10266           
=======================================
  Hits        10042    10042           
  Misses        156      156           
  Partials       68       68           
Impacted Files Coverage Δ
github/repos.go 98.70% <ø> (ø)

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 51df45c...d801426. Read the comment docs.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @k24dizzle !
Just a couple tweaks, please.

github/repos.go Outdated Show resolved Hide resolved
github/repos.go Outdated Show resolved Hide resolved
@gmlewis
Copy link
Collaborator

gmlewis commented Jan 29, 2022

It looks like the linter has a couple issues as well.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @k24dizzle !
LGTM.
Merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants