Split pull request search from issue; extend with filters by kgromov · Pull Request #1693 · hub4j/github-api

kgromov · 2023-08-11T21:23:37Z

Enhancement

Add pull request search implementation to split from issues - GHPullRequestSearchBuilder
Extend with most of search parameters according to documentation
Added to both GHRepository and GitHub client itself - so it's possible to search for pull requests from certain repository or in general for organization: GHRepository#searchPullRequests(GHPullRequestSearchBuilder search) and GitHub#searchPullRequests accordingly.

Inspiration for this extension was own project with custom filters grouped differently to what github UI prvides (and a bit more user intuitive). And as it turned out library did not have a lot of search parameters in order to shorten search and response time (like created/merged dates and ranges, labels etc).

kgromov · 2023-08-16T21:42:59Z

What I'm doing wrong? Since on my local env mvn spotless:apply build is successful for both java11 & 17:

bitwiseman · 2023-08-16T21:48:19Z

Did you commit and push any updates? I just did so.

src/main/java/org/kohsuke/github/GHPullRequestSearchBuilder.java

kgromov · 2023-08-16T22:14:32Z

Did you commit and push any updates? I just did so.

Sorry my bad - didn't notice those changes in javadoc

src/main/java/org/kohsuke/github/GHPullRequestSearchBuilder.java

src/test/java/org/kohsuke/github/GHRepositoryTest.java

src/test/java/org/kohsuke/github/AppTest.java

bitwiseman · 2023-08-18T20:51:17Z

@kgromov
Looks like there's some test failures now.

kgromov · 2023-08-18T21:40:01Z

@kgromov Looks like there's some test failures now.

I'll take a look.
New tests were passed in -Dtest.github.useProxy and -Dtest.github.org=false -Dtest.github.takeSnapshot modes respectively.
Should I request access to hub4j-test-org or it's not necessary and snapshots can be taken from own repository?

bitwiseman · 2023-08-19T04:10:43Z

Snapshots can be taken from anywhere, but we prefer the test org for later rerecording if needed.

It shouldn't cause failures though.

bitwiseman · 2023-08-19T04:11:43Z

Again, you need to check in files created during takeSnapshot.

…estPullRequestsSearch

kgromov · 2023-08-24T21:19:21Z

Again, you need to check in files created during takeSnapshot.

@bitwiseman can you please trigger build - I've updated snapshots few days ago

kgromov · 2023-08-25T22:23:01Z

Again, you need to check in files created during takeSnapshot.

@bitwiseman can you please trigger build - I've updated snapshots few days ago

And again please (hopefully last time) - I missed one obvious part that wasn't specified in the CONTRIBUTING.md 😊

bitwiseman · 2023-08-25T22:27:18Z

Please update the contributing doc with the instructions that were missing. Thanks!

…nal repository

kgromov · 2023-08-25T23:00:28Z

Please update the contributing doc with the instructions that were missing. Thanks!

Done (with test data fix)

kgromov · 2023-08-31T23:05:08Z

Hi, @bitwiseman.
Following existing pattern I've added org.kohsuke.github.GHPullRequestSearchBuilder to exclusion for code coverage (since it's also has setters for complex GH* objects) and implies interaction with Requester:

<exclude>org.kohsuke.github.GHPullRequestReviewBuilder.DraftReviewComment</exclude>
                      <exclude>org.kohsuke.github.GHIssue.PullRequest</exclude>
                      <exclude>org.kohsuke.github.GHCommitSearchBuilder</exclude>
                      <exclude>org.kohsuke.github.GHRepositorySearchBuilder</exclude>
                      <exclude>org.kohsuke.github.GHUserSearchBuilder</exclude>

Is it fine or should I cover all methods for new class?

bitwiseman · 2023-09-01T00:45:35Z

I'll take a look shortly.

codecov · 2023-09-08T00:32:35Z

Codecov Report

Attention: 8 lines in your changes are missing coverage. Please review.

Files	Coverage Δ
src/main/java/org/kohsuke/github/GHRepository.java	`72.82% <100.00%> (+1.07%)`	⬆️
src/main/java/org/kohsuke/github/GitHub.java	`82.89% <100.00%> (+0.07%)`	⬆️
...org/kohsuke/github/GHPullRequestSearchBuilder.java	`91.11% <91.11%> (ø)`

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!

kgromov · 2023-09-22T20:50:28Z

@bitwiseman, can you trigger builds? I guess last commit was fine (I extended code coverage to 91%) - I just forgot, that java.version is 8

kgromov · 2023-11-05T23:52:43Z

Hi, @bitwiseman. Any news when PR can be merged, thanks?

src/main/java/org/kohsuke/github/GHPullRequestSearchBuilder.java

src/main/java/org/kohsuke/github/GHRepository.java

bitwiseman

Great job on the test coverage!
This is very close. Last couple questions.

kgromov · 2023-11-11T00:57:38Z

Great job on the test coverage! This is very close. Last couple questions.

Updated - please take a look last commit

…es compatibility

kgromov · 2023-11-12T21:18:16Z

Great job on the test coverage! This is very close. Last couple questions.

Updated - please take a look last commit

UPD: rolled back changed type for terms. I would personally make it LinkedHashSe to guarantee terms uniqueness (hopefully it's not affected github API but can add some ambiguity for wirewock mappings):

public abstract class GHSearchBuilder<T> extends GHQueryBuilder<T> {

    /** The terms. */
    protected final List<String> terms = new ArrayList<String>();

bitwiseman · 2023-11-13T20:38:55Z

Great job on the test coverage! This is very close. Last couple questions.

Updated - please take a look last commit

UPD: rolled back changed type for terms. I would personally make it LinkedHashSe to guarantee terms uniqueness (hopefully it's not affected github API but can add some ambiguity for wirewock mappings):
public abstract class GHSearchBuilder<T> extends GHQueryBuilder<T> {

    /** The terms. */
    protected final List<String> terms = new ArrayList<String>();

That seems like a reasonable change, but maybe make that in a separate PR after we land this one?

src/main/java/org/kohsuke/github/GHPullRequestSearchBuilder.java

src/main/java/org/kohsuke/github/GHRepository.java

kgromov · 2023-11-14T21:39:12Z

Great job on the test coverage! This is very close. Last couple questions.

Updated - please take a look last commit

UPD: rolled back changed type for terms. I would personally make it LinkedHashSe to guarantee terms uniqueness (hopefully it's not affected github API but can add some ambiguity for wirewock mappings):
public abstract class GHSearchBuilder<T> extends GHQueryBuilder<T> {

    /** The terms. */
    protected final List<String> terms = new ArrayList<String>();
That seems like a reasonable change, but maybe make that in a separate PR after we land this one?

yes, since it's related to all search builders

Split pull request search from issue; extend with filters

d9c5958

Apply spotless updates

e145af8

Merge branch 'main' into feature/extend-pull-requests-query-search

eec7479

bitwiseman reviewed Aug 16, 2023

View reviewed changes

src/main/java/org/kohsuke/github/GHPullRequestSearchBuilder.java Show resolved Hide resolved

bitwiseman reviewed Aug 16, 2023

View reviewed changes

src/main/java/org/kohsuke/github/GHPullRequestSearchBuilder.java Outdated Show resolved Hide resolved

Apply suggestions from code review

bdc3866

bitwiseman reviewed Aug 17, 2023

View reviewed changes

src/main/java/org/kohsuke/github/GHPullRequestSearchBuilder.java Outdated Show resolved Hide resolved

Update src/main/java/org/kohsuke/github/GHPullRequestSearchBuilder.java

4d02a3c

bitwiseman reviewed Aug 17, 2023

View reviewed changes

src/test/java/org/kohsuke/github/GHRepositoryTest.java Show resolved Hide resolved

Update src/test/java/org/kohsuke/github/GHRepositoryTest.java

321b6ea

bitwiseman reviewed Aug 17, 2023

View reviewed changes

src/test/java/org/kohsuke/github/AppTest.java Show resolved Hide resolved

Update src/test/java/org/kohsuke/github/AppTest.java

8b59aa4

Update snapshots fot GHRepository#testPullRequestSearch and AppTest#t…

c355a14

…estPullRequestsSearch

bitwiseman and others added 2 commits August 24, 2023 14:25

Merge branch 'main' into feature/extend-pull-requests-query-search

02325be

Change target to wiremock files explicitly to custom repo

e933b4c

kgromov added 2 commits August 26, 2023 01:38

Fix creationDate timezone issue for test data

7d8ca89

Update CONTRIBUTING.md with small tips for snapshots taken from perso…

f2c5149

…nal repository

kgromov added 2 commits August 28, 2023 00:26

Fix created search param caused by timezone diff

9e03957

Fix code coverage

74f407a

kgromov and others added 2 commits September 13, 2023 01:19

Cover pull request search builder more than 90%

7c98382

Merge branch 'main' into feature/extend-pull-requests-query-search

4c613ba

kgromov requested a review from bitwiseman September 12, 2023 22:21

Fix for java 8 build - remove factory method for Set

dc0a8fa

Merge branch 'main' into feature/extend-pull-requests-query-search

ea17ade

bitwiseman reviewed Nov 7, 2023

View reviewed changes

src/main/java/org/kohsuke/github/GHPullRequestSearchBuilder.java Outdated Show resolved Hide resolved

bitwiseman reviewed Nov 7, 2023

View reviewed changes

src/main/java/org/kohsuke/github/GHRepository.java Show resolved Hide resolved

bitwiseman requested changes Nov 7, 2023

View reviewed changes

kgromov and others added 2 commits November 11, 2023 00:26

Merge branch 'main' into feature/extend-pull-requests-query-search

e0cb1b2

Change GHRepository#searchPullRequests to return buider

8d9b194

kgromov and others added 2 commits November 12, 2023 00:52

Merge branch 'main' into feature/extend-pull-requests-query-search

0859587

Rollback optimimzation for unique search terms to satisfy japicmp typ…

a82c15e

…es compatibility

bitwiseman reviewed Nov 13, 2023

View reviewed changes

src/main/java/org/kohsuke/github/GHPullRequestSearchBuilder.java Outdated Show resolved Hide resolved

bitwiseman reviewed Nov 13, 2023

View reviewed changes

src/main/java/org/kohsuke/github/GHRepository.java Outdated Show resolved Hide resolved

Remove extra constructor from GHPullRequestSearchBuilder

586f89e

bitwiseman approved these changes Nov 15, 2023

View reviewed changes

bitwiseman merged commit 9ff3264 into hub4j:main Nov 15, 2023

Conversation

kgromov commented Aug 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kgromov commented Aug 16, 2023

Uh oh!

bitwiseman commented Aug 16, 2023

Uh oh!

Uh oh!

Uh oh!

kgromov commented Aug 16, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bitwiseman commented Aug 18, 2023

Uh oh!

kgromov commented Aug 18, 2023

Uh oh!

bitwiseman commented Aug 19, 2023

Uh oh!

bitwiseman commented Aug 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kgromov commented Aug 24, 2023

Uh oh!

kgromov commented Aug 25, 2023

Uh oh!

bitwiseman commented Aug 25, 2023

Uh oh!

kgromov commented Aug 25, 2023

Uh oh!

kgromov commented Aug 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bitwiseman commented Sep 1, 2023

Uh oh!

codecov bot commented Sep 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kgromov commented Sep 22, 2023

Uh oh!

kgromov commented Nov 5, 2023

Uh oh!

Uh oh!

Uh oh!

bitwiseman left a comment

Choose a reason for hiding this comment

Uh oh!

kgromov commented Nov 11, 2023

Uh oh!

kgromov commented Nov 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bitwiseman commented Nov 13, 2023

Uh oh!

Uh oh!

Uh oh!

kgromov commented Nov 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kgromov commented Aug 11, 2023 •

edited

Loading

bitwiseman commented Aug 19, 2023 •

edited

Loading

kgromov commented Aug 31, 2023 •

edited

Loading

codecov bot commented Sep 8, 2023 •

edited

Loading

kgromov commented Nov 12, 2023 •

edited

Loading