Refactor CodableRouter to ease code reuse#1222
Merged
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1222 +/- ##
==========================================
+ Coverage 87.93% 89.38% +1.44%
==========================================
Files 38 38
Lines 2404 2232 -172
==========================================
- Hits 2114 1995 -119
+ Misses 290 237 -53
Continue to review full report at Codecov.
|
djones6
requested changes
Feb 27, 2018
Collaborator
djones6
left a comment
There was a problem hiding this comment.
This refactor looks great. Please could you add doc for the new helper functions?
Collaborator
Author
|
@djones6 Good point! Done. |
djones6
reviewed
Feb 28, 2018
| * If successful, the HTTP status code will be set to `HTTPStatusCode.noContent` and no | ||
| * body will be sent. | ||
| * | ||
| * If failed, the HTTP status code used for the response wll be set to either the |
djones6
reviewed
Feb 28, 2018
| * | ||
| * - Parameter from: The RequestError to map to a HTTPStatusCode | ||
| * - Returns: A HTTPStatusCode corresponding to the RequestError http code | ||
| * if valid, or HTTPStatusCode.unknown otherwise |
Collaborator
There was a problem hiding this comment.
Nitpick: Should the code elements here be back-ticked?
Collaborator
Author
There was a problem hiding this comment.
Yes! I just missed them. :(
djones6
approved these changes
Feb 28, 2018
Collaborator
|
Thank you for adding this excellent documentation. Let's get this merged and the (tiny) nitpicks can be addressed later. |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Remove code duplication in the CodableRouter and expose CodableHelpers as building blocks for extensions to the CodableRouter.
Motivation and Context
While working on a prototype codable authentication extension to the router I needed to write variations of the existing route registration functions from CodableRouter. These functions have a lot of code duplication to begin with, and with this new extension each function would need to be duplicated again to add the variations.
This motivated a refactor of these functions so they could be constructed from building block functions. These could then be exposed for use by router extensions.
How Has This Been Tested?
Automated testing
Checklist: