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

Sort Support for feign pagination #328

Closed
wants to merge 3 commits into from
Closed

Conversation

cbezmen
Copy link
Contributor

@cbezmen cbezmen commented Apr 28, 2020

PageJacksonModule doesn't support for sort operations.

@codecov
Copy link

codecov bot commented Apr 28, 2020

Codecov Report

Merging #328 into master will increase coverage by 0.10%.
The diff coverage is 83.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #328      +/-   ##
============================================
+ Coverage     79.09%   79.19%   +0.10%     
- Complexity      351      356       +5     
============================================
  Files            42       44       +2     
  Lines          1320     1360      +40     
  Branches        202      205       +3     
============================================
+ Hits           1044     1077      +33     
- Misses          190      196       +6     
- Partials         86       87       +1     
Impacted Files Coverage Δ Complexity Δ
...ork/cloud/openfeign/support/SortJsonComponent.java 73.07% <73.07%> (ø) 0.00 <0.00> (?)
...ork/cloud/openfeign/FeignClientsConfiguration.java 97.14% <100.00%> (+0.08%) 14.00 <1.00> (+1.00)
...ork/cloud/openfeign/support/PageJacksonModule.java 59.25% <100.00%> (+5.09%) 4.00 <0.00> (ø)
...ork/cloud/openfeign/support/SortJacksonModule.java 100.00% <100.00%> (ø) 4.00 <4.00> (?)

Copy link
Contributor

@ryanjbaxter ryanjbaxter left a comment

Choose a reason for hiding this comment

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

This would make sense to go in a Hoxton service release right? If so submit the PR against the 2.2.x branch

@ryanjbaxter
Copy link
Contributor

Also be sure to add some documentation that this would now be supported

@cbezmen cbezmen changed the base branch from master to 2.2.x April 29, 2020 05:25
@cbezmen cbezmen changed the base branch from 2.2.x to master April 29, 2020 05:28
@cbezmen
Copy link
Contributor Author

cbezmen commented Apr 29, 2020

Hi Ryan,
I am new to this. Could you help me about documentation. Where should I write the documentation. etc.
@ryanjbaxter

@cbezmen
Copy link
Contributor Author

cbezmen commented Apr 29, 2020

I created new PR for new branch 2.2.x.
#329

@OlgaMaciaszek
Copy link
Collaborator

@cbezmen The documentation should be added to spring-cloud-openfeign.adoc.

Closing this PR in favour of #329

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.

None yet

4 participants