github-api
github-api copied to clipboard
Added listGists method
Description
Currently, when pulling down a list of GHGist objects, using
gitHub.getMyself().listGists().toList()
The GHGist objects returned in the list do not contain the contents of the files within each Gist, and there is no other method from which to pull down all gists with complete files in a single list. The reason for this seems to be that the GitHub API URL path that is used to pull the list from, intentionally does not include the contents of the file. The API docs seem to indicate a conservative data yield for that particular path, requiring a developer to then use another URL path from which to pull the individual Gists, which would then include the contents of the file.
I added this method to the GitHub class, which I have tested and verified that the list it returns is a complete list of the user's GHGist objects, where each file within the Gists has all of the data associated with the file, including content.
Fixes #1325
The only issue that came from mvn -D enable-ci clean install site was a warning about inconsistent synchronization, but it didn't just flag my code, it flagged other code in the class as well. It also indicates that the code might be just fine and that it could be a false positive due to the nature of the test.
The method is relatively simple, I cannot see where it would cause any issues. However, I'm willing to make whatever changes are necessary.
Before submitting a PR:
- [X] Changes must not break binary backwards compatibility. If you are unclear on how to make the change you think is needed while maintaining backward compatibility, CONTRIBUTING.md for details.
- [X] Add JavaDocs and other comments as appropriate. Consider including links in comments to relevant documentation on https://docs.github.com/en/rest .
- [ ] Add tests that cover any added or changed code. This generally requires capturing snapshot test data. See CONTRIBUTING.md for details.
- [X] Run
mvn -D enable-ci clean install sitelocally. If this command doesn't succeed, your change will not pass CI. - [X] Push your changes to a branch other than
main. You will create your PR from that branch.
When creating a PR:
- [X] Fill in the "Description" above with clear summary of the changes. This includes:
- [X] If this PR fixes one or more issues, include "Fixes #
" lines for each issue. - [ ] Provide links to relevant documentation on https://docs.github.com/en/rest where possible.
- [X] If this PR fixes one or more issues, include "Fixes #
- [ ] All lines of new code should be covered by tests as reported by code coverage. Any lines that are not covered must have PR comments explaining why they cannot be covered. For example, "Reaching this particular exception is hard and is not a particular common scenario."
- [X] Enable "Allow edits from maintainers".
Codecov Report
Merging #1490 (aad672c) into main (262cf84) will decrease coverage by
0.06%. The diff coverage is0.00%.
@@ Coverage Diff @@
## main #1490 +/- ##
============================================
- Coverage 78.81% 78.75% -0.07%
Complexity 2114 2114
============================================
Files 202 202
Lines 6429 6434 +5
Branches 361 362 +1
============================================
Hits 5067 5067
- Misses 1152 1157 +5
Partials 210 210
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/main/java/org/kohsuke/github/GitHub.java | 80.35% <0.00%> (-1.84%) |
:arrow_down: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Please add at least one test for your new method.
The test will need to use an account that has Gists and files in the Gists in order to work. I wrote a test that authenticates with my own token and it works, but I don't know how to write the test so that it will always work regardless of who is running it.
This test works, but it will only work when a valid token has been given to it.
@Test
public void gistListWithFilesTest() throws Exception {
GitHub gh = new GitHubBuilder().withOAuthToken("<valid token>").build();
List<GHGist> ghGistList = gh.listGists();
assertThat(ghGistList, notNullValue());
for(GHGist ghGist : ghGistList) {
assertThat(ghGist.getFiles(), notNullValue());
for(GHGistFile gistFileContent : ghGist.getFiles().values()) {
boolean hasContent = gistFileContent.getContent().length() > 0;
assertThat(hasContent, is(true));
}
}
}
@EasyG0ing1
You can record a snapshot of the tests and then add the snapshotNotAllowed(); method to prevent it from being overwritten later.
However, I'm going o make another suggestion. Instead of adding a new method to get the list Gists with contents, I suggest you add a populate() method to GHGist that calls the URL that returns the full contents similar to other populate() methods:
https://github.com/hub4j/github-api/blob/bcb71a3c318d243c92b757310cf153b8afc983d5/src/main/java/org/kohsuke/github/GHLicense.java#L196-L199 That will allow people to get the file contents if needed while preserving the incremental loading. The test will also be simpler. You can add a step at the end of the existing test for listing Gists.