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

fix: use new media type for branch protections #881

Merged
merged 3 commits into from Jun 11, 2020

Conversation

iBotPeaches
Copy link
Contributor

Not sure when this changed, but loki is not the preview anymore. It is luke-cage. Without this fix you won't get back branch protection values. (like required amount of reviewers).

https://developer.github.com/v3/repos/branches/#get-branch-protection

If you need to support commit signature crud, that has a different media type under Protections, but I don't see that supported at this time.

@acrobat
Copy link
Collaborator

acrobat commented Jun 11, 2020

@iBotPeaches thanks for the pullrequest! But can you change the code so that we set the required acceptHeaderValue in the specific methods instead? This way users don't need to call the configure method and it would resolve issues with multiple preview modes for api endpoints in the same class.

There are 4 endpoint that would need this acceptHeaderValue: https://developer.github.com/changes/2018-03-16-protected-branches-required-approving-reviews/

Example:

public function update($username, $repository, $checkRunId, array $params = [])
{
// This api is in preview mode, so set the correct accept-header.
$this->acceptHeaderValue = 'application/vnd.github.antiope-preview+json';
return $this->patch('/repos/'.rawurlencode($username).'/'.rawurlencode($repository).'/check-runs/'.rawurlencode($checkRunId), $params);
}

@iBotPeaches
Copy link
Contributor Author

@acrobat - I could not find 2 of the endpoints supported/built, so only patched the two that matched in the list you provided.

Should I revert the change to configure() ?

@acrobat
Copy link
Collaborator

acrobat commented Jun 11, 2020

@acrobat - I could not find 2 of the endpoints supported/built, so only patched the two that matched in the list you provided.

Great 👌

Should I revert the change to configure() ?

Yeah, revert it for now!

@acrobat acrobat merged commit d557dc4 into KnpLabs:master Jun 11, 2020
@acrobat
Copy link
Collaborator

acrobat commented Jun 11, 2020

Thanks @iBotPeaches! And congrats on your first contribution! 🎉

@iBotPeaches iBotPeaches deleted the fix-protections branch June 11, 2020 21:17
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.

None yet

2 participants