Skip to content

fix: use new media type for branch protections - #881

Merged
acrobat merged 3 commits into
KnpLabs:masterfrom
iBotPeaches:fix-protections
Jun 11, 2020
Merged

fix: use new media type for branch protections#881
acrobat merged 3 commits into
KnpLabs:masterfrom
iBotPeaches:fix-protections

Conversation

@iBotPeaches

Copy link
Copy Markdown
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

acrobat commented Jun 11, 2020

Copy link
Copy Markdown
Collaborator

@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
Copy Markdown
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

acrobat commented Jun 11, 2020

Copy link
Copy Markdown
Collaborator

@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

acrobat commented Jun 11, 2020

Copy link
Copy Markdown
Collaborator

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.

2 participants