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

Added missing Apps endpoints #814

Merged
merged 8 commits into from Nov 14, 2019

Conversation

jamiehuson
Copy link
Contributor

Why make this change?
Some endpoints available on https://developer.github.com/v3/apps/ are not available through this package. I also added the Accept header back for the preview Apps apis, which addresses this issue: #800

Considerations
I did not include Create Github App from App-Manifest because that flow required additional scoping to fully understand the flow, which was beyond the scope of my needs at this time.

Testing
I have included tests for all added endpoints.

lib/Github/Api/Apps.php Outdated Show resolved Hide resolved
@acrobat
Copy link
Collaborator

acrobat commented Nov 1, 2019

Small change on the configure method requested, the rest of the PR looks good!

@acrobat
Copy link
Collaborator

acrobat commented Nov 1, 2019

@jamiehuson can you rebase you PR against the current master? And use the configurePreviewHeader method where we need the set a custom accept header? It was added in #823

Copy link
Collaborator

@acrobat acrobat left a comment

Choose a reason for hiding this comment

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

2 small remarks, the rest of the PR looks good! 👍

lib/Github/Api/Apps.php Outdated Show resolved Hide resolved
lib/Github/Api/Apps.php Outdated Show resolved Hide resolved
@acrobat
Copy link
Collaborator

acrobat commented Nov 14, 2019

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

@acrobat acrobat merged commit 4018a93 into KnpLabs:master Nov 14, 2019
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