Skip to content
This repository has been archived by the owner on Sep 28, 2022. It is now read-only.

remove fetchAllDocuments route #134

Merged
merged 1 commit into from
Jun 7, 2017

Conversation

benoitvidis
Copy link
Contributor

This PR removes the fetchAllDocuments method from the SDK, which can be better replaced by a search + fetchNext call (documentation to come).

@codecov-io
Copy link

codecov-io commented Jun 6, 2017

Codecov Report

Merging #134 into 3.x will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              3.x     #134      +/-   ##
==========================================
+ Coverage   91.34%   91.38%   +0.04%     
==========================================
  Files          33       33              
  Lines        4148     4320     +172     
  Branches      397      419      +22     
==========================================
+ Hits         3789     3948     +159     
- Misses        255      264       +9     
- Partials      104      108       +4
Impacted Files Coverage Δ
src/main/java/io/kuzzle/sdk/core/Collection.java 96.89% <ø> (-0.16%) ⬇️
...ain/java/io/kuzzle/sdk/responses/SearchResult.java 55.55% <0%> (-5.56%) ⬇️
src/main/java/io/kuzzle/sdk/security/User.java 96.15% <0%> (+0.19%) ⬆️
src/main/java/io/kuzzle/sdk/security/Security.java 93.19% <0%> (+0.24%) ⬆️

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 35d807f...0cf82b2. Read the comment docs.

Copy link
Contributor

@scottinet scottinet left a comment

Choose a reason for hiding this comment

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

You only removed a fetchAllDocuments overload, the main implementation is still there.

@benoitvidis benoitvidis force-pushed the ref-23-remove-fetchalldocuments branch from 0555139 to 0cf82b2 Compare June 7, 2017 12:10
@benoitvidis
Copy link
Contributor Author

oops.. -_-;... fixed

this.fetchAllDocuments(new Options(), listener);
}

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

You only removed a signature of fetchAlldocuments, the actual method doing the job is still just below :)

@stafyniaksacha stafyniaksacha merged commit aeda174 into 3.x Jun 7, 2017
@stafyniaksacha stafyniaksacha deleted the ref-23-remove-fetchalldocuments branch June 7, 2017 13:45
@scottinet scottinet mentioned this pull request Jun 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants