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

[JENKINS-68809] Topic repository listing should include forks by default #580

Merged
merged 4 commits into from
Jun 30, 2022

Conversation

Dohbedoh
Copy link
Contributor

@Dohbedoh Dohbedoh commented Jun 23, 2022

Description

JENKINS-68809: the recent Topic filter added does not include fork by default. Due to the default behavior of the Search API. It probably should given that there is now an ExcludeForkedRepositoriesTrait.

This PR add fork:true to the search unless ExcludeForkedRepositoriesTrait is added.

Note: we could actually always add fork:true and let the API filter out the results. But by using the filter in the search query, we can ave on that computation.

Submitter checklist

  • Link to JIRA ticket in description, if appropriate.
  • Change is code complete and matches issue description
  • Automated tests have been added to exercise the changes
  • Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.

Reviewer checklist

  • Run the changes and verify that the change matches the issue description
  • Reviewed the code
  • Verified that the appropriate tests have been written or valid explanation given

Documentation changes

  • Link to jenkins.io PR, or an explanation for why no doc changes are needed

Users/aliases to notify

@jenkinsci/github-branch-source-plugin-developers

@rsandell rsandell added the bug label Jun 30, 2022
@rsandell
Copy link
Member

@Dohbedoh To you see this as a bugfix or a feature/enhancement? For me it's a bit of a gray area :)

@Dohbedoh
Copy link
Contributor Author

I was thinking of it as a bug given the facts that the event mechanism considers forks, which is inconsistent

@rsandell rsandell merged commit 35b3b02 into jenkinsci:master Jun 30, 2022
@Dohbedoh Dohbedoh deleted the bugfix/JENKINS-68809 branch June 30, 2022 23:23
topics.forEach(ghRepositorySearchBuilder::topic);
ghRepositorySearchBuilder.q("org:" + getRepoOwner());
context.getTopics().forEach(ghRepositorySearchBuilder::topic);
ghRepositorySearchBuilder.org(getRepoOwner());
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for updating this :)

I had put a pull request into github api a while ago

hub4j/github-api#1355

@joshbranham
Copy link

Looks like this resolves https://issues.jenkins.io/browse/JENKINS-68765 thank you!

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

Successfully merging this pull request may close these issues.

4 participants