Fixes #183: added a method listForks() to GHRepository #185
Conversation
|
Kohsuke Kawaguchi » github-api #338 SUCCESS |
listForks() will list all forks of a repository. An optional sort argument is also supported.
| /** | ||
| * Lists up all the forks of this repository, sorted by the given sort order. | ||
| */ | ||
| public PagedIterable<GHRepository> listForks(final Sort sort) { |
There was a problem hiding this comment.
Add @CheckForNull and describe the behavior in Javadoc
There was a problem hiding this comment.
There is not a single @CheckForNull in the entire codebase so far. If you want to start introducing them, go ahead, but that was not the scope of this pull request.
Regarding Javadoc, I was under the impression that both methods I added actually have Javadoc comments, but maybe I miscounted.
There was a problem hiding this comment.
My intention was to highlight the method accepts nulls and to describe the behavior in that case.
There is not a single @checkfornull in the entire codebase so far. If you want to start introducing them, go ahead, but that was not the scope of this pull request.
I think that new pull requests could follow better practices than the legacy code
There was a problem hiding this comment.
Agreed.
Just added (hopefully) better Javadoc. Unfortunately, I was not able to find out how to add the @CheckForNull annotation.
|
Kohsuke Kawaguchi » github-api #348 SUCCESS |
|
Any chance of merging this? We are using this all the time now, and not a single problem so far... |
| public PagedIterable<GHRepository> listForks(final Sort sort) { | ||
| return new PagedIterable<GHRepository>() { | ||
| public PagedIterator<GHRepository> iterator() { | ||
| return new PagedIterator<GHRepository>(root.retrieve().asIterator(getApiTailUrl("forks" + ((sort == null)?"":("?sort="+sort.toString().toLowerCase(Locale.ENGLISH)))), GHRepository[].class)) { |
There was a problem hiding this comment.
(sort == null)?"":("?sort="+sort.toString().toLowerCase(Locale.ENGLISH))))
this line is too difficult to understand - can you use separate method for this logic?
There was a problem hiding this comment.
Actually it is quite easy, if a sort is given, use it, otherwise don't.
Sorry, but I don't know how to write this more clearly...
There was a problem hiding this comment.
- Spaces. Use them.
public PagedIterator<GHRepository> iterator() {
return new PagedIterator<GHRepository>(root.retrieve()
// statically imported org.apache.commons.lang.ObjectUtils.defaultIfNull
.with("sort", defaultIfNull(sort, Sort.NEWEST).name().toLowerCase())
.asIterator(getApiTailUrl("forks")), GHRepository[].class)) {Actually it is quite easy, if a sort is given, use it, otherwise don't.
If one line gets more than 10s to understand code - it need to be refactored
|
Kohsuke Kawaguchi » github-api #367 SUCCESS |
| @@ -37,6 +37,7 @@ | |||
| import java.util.*; | |||
|
|
|||
| import static java.util.Arrays.asList; | |||
| import static org.apache.commons.lang.ObjectUtils.defaultIfNull; | |||
There was a problem hiding this comment.
you dont need it if you dont use it :)
There was a problem hiding this comment.
Oh, lol, I put it in and then forgot to remove it again ;)
|
@marc-guenther, thanks for update, 👍 for me now |
Fixes #183: added a method listForks() to GHRepository
|
Kohsuke Kawaguchi » github-api #371 SUCCESS |
listForks() will list all forks of a repository.
An optional sort argument is also supported.
I did not know if list* or get* is the way to go, but as getForks() is already taken (it returns the number of forks), I decided to use listForks(), with an optional sort argument.