Convenience method to auth with app installation token && documentation examples #583
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Convert existing markdown doc to APT due to formatting features; Replace occurrences of kohsuke user to github-api org where applicable; Fix repositoryIds method on GHAppCreateTokenBuilder; Signed-off-by: PauloMigAlmeida <paulo.miguel.almeida.rodenas@gmail.com>
PauloMigAlmeida
changed the title
File issues or use the the wiki to track future ideas and work. I'll take a look at this shortly. |
bitwiseman
reviewed
Oct 30, 2019
bitwiseman
reviewed
Oct 30, 2019
| @@ -0,0 +1,154 @@ | |||
| What is this? | |||
There was a problem hiding this comment.
I'm going to talk through my thoughts here :
- I have no love of markdown - using something else works fine for me.
- I want to encourage you to make the docs better. From that perspective whatever format you want to use is fine by me.
- I also want others to be able to do the same. I hadn't heard of APT before this. Others might not know it either and would need to learn it (though it looks easy enough).
- I also want to be able to preview the changes on GitHub easily. This doesn't do that - we just get text. Bummer.
Meh, okay, let's try this.
bitwiseman
reviewed
Oct 30, 2019
bitwiseman
approved these changes
Oct 30, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi @bitwiseman
This PR fixes #570 #582 things discussed there.
List of things done:
Added a convenience method to authenticate with app installation tokens;
Converted existing markdown doc to APT due to formatting features;
The maven-site-plugin (even the 3.3 onwards) Markdown implementation is rather limited and it wasn't exactly clear, visually, when the code snippets started when I started adding more of them.
Before:
After:
Replaced occurrences of kohsuke user to github-api org where applicable;
I noticed that you transferred the github-api project to an org with the same name so I seized the opportunity to fix references in the source code to it too.
Fixed repositoryIds method on GHAppCreateTokenBuilder;
That was something that slipped through the cracks on my last PR that both of us didn't notice during the review. The type should be
Longrather thanInteger. Considering that you haven't released it yet, I thought that now was the perfect opportunity to fix it.In case you want me to deprecate the original method and create a second one with the right method signature instead of replacing it, let me now ... it's up to you tbh.
Future work:
I plan to create another documentation page with the steps related to deal with Github apps but I decided to first focus on the several ways people can authenticate to Github with this library before anything else (most recurrent question IMHO). Also, I needed to know if I am heading in the right direction that you guys have in mind.