Skip to content

Simplify creation of PagedIterables from requests#563

Merged
bitwiseman merged 3 commits intohub4j:masterfrom
bitwiseman:paged_iterable_closure
Oct 11, 2019
Merged

Simplify creation of PagedIterables from requests#563
bitwiseman merged 3 commits intohub4j:masterfrom
bitwiseman:paged_iterable_closure

Conversation

@bitwiseman
Copy link
Member

@bitwiseman bitwiseman commented Oct 7, 2019

This change eliminates a whole bunch of cruft from the creation of PagedIterables. Dependent on Java 8+ closures.

@bitwiseman bitwiseman marked this pull request as ready for review October 7, 2019 15:21
@bitwiseman bitwiseman changed the title Simplify creation of PagedIterables from reqests Simplify creation of PagedIterables from requests Oct 7, 2019
@bitwiseman bitwiseman force-pushed the paged_iterable_closure branch from d58f4d1 to 2fba184 Compare October 7, 2019 15:24
@bitwiseman bitwiseman requested a review from kohsuke October 7, 2019 15:28
@bitwiseman
Copy link
Member Author

@martinvanzijl
What do you think of this?

@martinvanzijl
Copy link
Contributor

@bitwiseman
I like it - it looks a lot neater!

Do you want me to do a formal review on GitHub? If so, just that one change for listMyAuthorizations() or all changes?

@bitwiseman
Copy link
Member Author

@martinvanzijl
Between your Authorization test and a few other tests that interact with PagedIterable, I'm pretty confident that the basic idea is sound. I've been exercising my admin power to get stuff in, but this is a significant enough change that I wanted at least one other person to look it over.

You've submitted a bunch of PRs and made tests. I feel you're familiar enough with the project to do that in a meaningful way.

Thanks!

@bitwiseman bitwiseman force-pushed the paged_iterable_closure branch from 4838607 to 281c927 Compare October 11, 2019 20:11
@bitwiseman bitwiseman merged commit 8ec861c into hub4j:master Oct 11, 2019
@bitwiseman bitwiseman deleted the paged_iterable_closure branch October 11, 2019 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants