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

Added response message examples support (#1570) #2514

Closed
wants to merge 51 commits into from

Conversation

solo-yolo
Copy link

Added new 'examples' property to ResponseMessage with corresponding
change in ResponseMessageBuilder.
Updated corresponding mapping logic, to populate response message
examples instead of an empty map.

It is related to issue #1570 and could be considered as the first step to adding response examples support.

PascalSchumacher and others added 29 commits June 16, 2018 13:54
complex cases were Optional.or.Optional and replacing presentInstances
FluentIterable.from still used and converted with toJavaUtil()
Need to ensure that Optional setting is correct in Defaults and ParameterRequiredReader
Are both java and guava Optional supported?
Blocked tests in FunctionContractSpec and DefaultRequestHandlerCombinerSpec, the fail on command line, work in IDE
still having on and off issues with DefaultRequestHandlerCombinerSpec and FunctionContractSpec
removed another test from FunctionContractSpec
removed more FuctionContractSpec tests
Added new 'examples' property to ResponseMessage with corresponding
change in ResponseMessageBuilder.
Updated corresponding mapping logic, to populate response message
examples instead of empty map.
@codecov
Copy link

codecov bot commented Jun 26, 2018

Codecov Report

Merging #2514 into master will decrease coverage by 0.04%.
The diff coverage is 95.13%.

@@             Coverage Diff              @@
##             master    #2514      +/-   ##
============================================
- Coverage     95.08%   95.03%   -0.05%     
- Complexity     2989     3059      +70     
============================================
  Files           339      340       +1     
  Lines          7949     7814     -135     
  Branches        643      596      -47     
============================================
- Hits           7558     7426     -132     
+ Misses          251      240      -11     
- Partials        140      148       +8

Also we use the sorted paths to get a consistent key for the same combination of paths regardless of order

(2516)

(cherry picked from commit 07a9225538465bc66d23fd2f41edd9ca5db75708)
(2516)

(cherry picked from commit ebeb20da28d39ee0e409cad0137fc2e1149ce95e)
Also simplified to use java 8 where possible.

(1082)
@solo-yolo
Copy link
Author

Hello, @dilipkrish! Any updates on this?

@dilipkrish
Copy link
Member

The PR needs conflicts resolved, but I was going pull these in shortly

@solo-yolo
Copy link
Author

Any actions required from my (our) side?
Or you gonna do it on your own?

Added new 'examples' property to ResponseMessage with corresponding
change in ResponseMessageBuilder.
Updated corresponding mapping logic, to populate response message
examples instead of empty map.
@solo-yolo
Copy link
Author

Seems like I've done it in a wrong way with rebase, sorry for that, it's actually my first PR :|

@dilipkrish
Copy link
Member

@q1nt no worries... I'll fix it up. For a first PR 👏 for putting discrete commits and splitting your work into incremental commits. What you probably did there was to not rebase against the origin branch.

When I work on PRs I find it easier to understand when I name my upstreams accordingly. For e.g. I usually name springfox/springfox as the origin upstream, and my fork as dilipkrish, that way its easy to remember which upstream is mine vs. which upstream is the repo Im sending a pull request for. Let me know if you have any questions. Id be happy to walk you through it.

@dilipkrish dilipkrish added the PR label Jul 3, 2018
@solo-yolo solo-yolo deleted the feature/response-examples branch July 13, 2018 10:44
@dilipkrish
Copy link
Member

@q1nt thanks for the PR. Sorry I took the easier route than fixing up this PR, which is essentially very similar.

@dilipkrish dilipkrish added this to the 3.0 milestone Jul 13, 2018
@solo-yolo
Copy link
Author

@dilipkrish
No problem!

Is there any chance those changes will be available as part of 2.9.3 version?

@dilipkrish
Copy link
Member

No unfortunately, it'll be part of 3.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants