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

fix nextpage logic and add additionalParameters logic and example #23

Conversation

tnrich
Copy link

@tnrich tnrich commented May 3, 2019

Let me know if you have questions. This fixed the issue I was seeing - #21

@codecov
Copy link

codecov bot commented May 3, 2019

Codecov Report

Merging #23 into master will increase coverage by 0.49%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #23      +/-   ##
==========================================
+ Coverage   49.01%   49.51%   +0.49%     
==========================================
  Files           1        1              
  Lines         102      103       +1     
  Branches       19       16       -3     
==========================================
+ Hits           50       51       +1     
  Misses         52       52
Impacted Files Coverage Δ
app.js 49.51% <71.42%> (+0.49%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cda42f8...43bc075. Read the comment docs.

@pavelbinar
Copy link
Member

Thank you very much for the PR!

I had an PR from @jeremysuriel recently which was fixing the pagination as well.
See here: https://github.com/remoteorigin/git-issues-downloader/pull/20/files

I think his changes are elegant and perhaps more flexible, like: https://github.com/remoteorigin/git-issues-downloader/pull/20/files#diff-0364f57fbff2fabbe941ed20c328ef1aR123

Lest's combine your and @jeremysuriel code together to bring best of both.

I will have time to do so by start of the next week. If you would like to proceed sooner feel free to do so; just extend this PR with Jeremys code.

@pavelbinar pavelbinar changed the base branch from master to develop May 4, 2019 11:58
@pavelbinar
Copy link
Member

Please see the updated develop

@pavelbinar pavelbinar removed the bug label May 4, 2019
@tnrich tnrich closed this Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants