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

allow including results from forks #1154

Closed
maxandersen opened this issue May 25, 2021 · 10 comments
Closed

allow including results from forks #1154

maxandersen opened this issue May 25, 2021 · 10 comments

Comments

@maxandersen
Copy link

Describe the bug
searchContent() does not seem to have a flag/method to enable searching content in forks, i.e. what corresponds to choosing "Fork" in the "Filters" section when searching, i.e.:

https://github.com/search?q=topic%3Ajbang-appstore+fork%3Atrue&type=repositories

Expected behavior
Ability to have search include results in forks.

@akashRindhe
Copy link
Contributor

akashRindhe commented May 25, 2021

There's an option in GHContentSearchBuilder to specify the fork query values, with the acceptable values being true and only according to the docs.

/**
* Fork gh content search builder.
*
* @param v
* the v
* @return the gh content search builder
*/
public GHContentSearchBuilder fork(String v) {
return q("fork:" + v);
}

@maxandersen
Copy link
Author

Ha. I thought that 'v' meant something else. Would have been good if the javsdocs itself stated what values was expected :)

@akashRindhe
Copy link
Contributor

Agreed that the docs could be a lot better. Additionally, an enum specifying the acceptable values instead of a String would make it self explanatory.

@maxandersen
Copy link
Author

so...just tried this now and it is not giving any additional results:

gitHub.searchContent().fork("true")..

also tried with "only" and still no results.

@bitwiseman
Copy link
Member

bitwiseman commented May 26, 2021

@maxandersen @akashRindhe

We have one test method currently:

@Test
public void searchContent() throws Exception {
PagedSearchIterable<GHContent> r = gitHub.searchContent()
.q("addClass")
.in("file")
.language("js")
.repo("jquery/jquery")
// ignored unless sort is also set
.order(GHDirection.DESC)
.list();
GHContent c = r.iterator().next();
// System.out.println(c.getName());
assertThat(c.getDownloadUrl(), notNullValue());
assertThat(c.getOwner(), notNullValue());
assertThat(c.getOwner().getFullName(), equalTo("jquery/jquery"));
assertThat(r.getTotalCount(), greaterThan(5));
PagedSearchIterable<GHContent> r2 = gitHub.searchContent()
.q("addClass")
.in("file")
.language("js")
.repo("jquery/jquery")
// resets query sort back to default
.sort(GHContentSearchBuilder.Sort.INDEXED)
.sort(GHContentSearchBuilder.Sort.BEST_MATCH)
// ignored unless sort is also set to non-default
.order(GHDirection.ASC)
.list();
GHContent c2 = r2.iterator().next();
assertThat(c2.getPath(), equalTo(c.getPath()));
assertThat(r2.getTotalCount(), equalTo(r.getTotalCount()));
PagedSearchIterable<GHContent> r3 = gitHub.searchContent()
.q("addClass")
.in("file")
.language("js")
.repo("jquery/jquery")
.sort(GHContentSearchBuilder.Sort.INDEXED)
.order(GHDirection.ASC)
.list();
GHContent c3 = r3.iterator().next();
assertThat(c3.getPath(), not(equalTo(c2.getPath())));
assertThat(r3.getTotalCount(), equalTo(r2.getTotalCount()));
PagedSearchIterable<GHContent> r4 = gitHub.searchContent()
.q("addClass")
.in("file")
.language("js")
.repo("jquery/jquery")
.sort(GHContentSearchBuilder.Sort.INDEXED)
.order(GHDirection.DESC)
.list();
GHContent c4 = r4.iterator().next();
assertThat(c4.getPath(), not(equalTo(c2.getPath())));
assertThat(c4.getPath(), not(equalTo(c3.getPath())));
assertThat(r4.getTotalCount(), equalTo(r2.getTotalCount()));
}

It looks like fork isn't covered. 😞 Test and possible fix welcome.

@sahansera
Copy link
Contributor

Hey @bitwiseman, I would like to look into this issue as well. Could you assign it to me? Thank you!

@sahansera
Copy link
Contributor

sahansera commented Nov 7, 2021

I had a closer look at the issue and I believe we are talking about 2 facets in here - i.e. Repository search and Code search.

  1. Repository search (type=Repositories)
    I was able to figure out a way to handle the use case explained by @maxandersen by using the searchRepositories() method.
var repos = github.searchRepositories()
        .q("jbang-appstore")
        .fork(GHRepositorySearchBuilder.Fork.PARENT_AND_FORKS)
        .order(GHDirection.DESC)
        .list();
repos.toList().forEach(ghRepository -> System.out.println(ghRepository.getFullName() + " " + ghRepository.isFork()));

This will output the following:

maxandersen/jbang-catalog false
gunnarmorling/jbang-catalog true

which matches with results given by this search URL.

  1. Code search (type=Code)
    GitHub doesn't index the content of forks if they don't have more stars than the parent repo. To quote the API docs:

Code in forks is only searchable if the fork has more stars than the parent repository. Forks with fewer stars than the parent repository are not indexed for code search.

This is why we don't get any results if we do a code search like this. I am happy to look at any sample use cases you might have that returns valid results.

However, I agree with what has been discussed here about refactoring the code to be able to use an enum for the searchContent().fork("") instead of passing strings which makes the code more self-explanatory. I am happy to do that change as part of this issue if you think that's beneficial.

Let me know what you think about this @bitwiseman

Cheers :)

@bitwiseman bitwiseman self-assigned this Nov 20, 2021
@sahansera
Copy link
Contributor

sahansera commented Nov 20, 2021

Based on the above comment I have created a draft PR to know your thoughts. I can target the hub4j repo if you are happy with it. PR - sahansera#1

I have submitted a PR for this: #1315

@bitwiseman
Copy link
Member

@sahansera
Thanks for the PR. The change looks basically good. I've commented.

@sahansera
Copy link
Contributor

@bitwiseman Thanks a lot for reviewing, improving and merging this PR. I really like the new GHQueryBuilder 🎉 It's been a great learning opportunity!

Are we able to close this issue as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants