Networking: Remote Infrastructure #54
Merged
Conversation
This file contains 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
jleandroperez
changed the title
jleandroperez
added
the
category: architecture
mindgraffiti
approved these changes
May 9, 2018
!|
|
||
| /// Date And Time Formatter | ||
| /// | ||
| static let dateTimeFormatter: DateFormatter = { |
There was a problem hiding this comment.
Is this not part of the WPShared pod? I thought it had several date helpers.
There was a problem hiding this comment.
Haven't really checked! but since it's a Woo-Backend format, it's okay to add a few lines here (vs linking an entire dependency!!). Goal is to keep this fellow as lightweight as possible ❤️
| } | ||
|
|
||
|
|
||
|
|
|
|
||
|
|
||
| /// OrderList Disposable Entity: | ||
| /// `Load All Orders` endpoint returns all of it's orders within the `data` key. This entity |
There was a problem hiding this comment.
Nitpicky!!! its instead of it's
| /// - Parameters: | ||
| /// - request: Request that should be performed. | ||
| /// - completion: Closure to be executed upon completion. | ||
| /// - completion: Closure to be executed upon completion. Will receive the JSON Parsed Response (if successfull) |
There was a problem hiding this comment.
Nitpicky - successful instead of successfull
|
Thanks A LOT for the review @mindgraffiti (all of the feedback addressed, thanks thanks thanks!!!!) |
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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:
This PR implements the basic Remote infrastructure we'll be using to call remote API's.
Ref. #24
Details:
Remote(this is analog to WPiOS).Remotecomes, from factory, with a newMappingmechanism, along with Unit Testing support (without requiring external dependencies).Order+OrderStatus+Address. This is not 100% finished, yet, and will be addressed in a third PR.Response Mapping:
This concept was envisioned by @diegoreymendez. The idea is all about defining the piece of code that parses Backend Responses, establishing a protocol with the expected API, and implementing "Mapping" objects, which will be responsible for parsing documents.
Huge benefits of this approach are, mostly:
MapperRemoteimplementation is no longer responsible for doing everything.Unit Testing
In Unit Testing Targets, we'll be able to access a
Remoteinternal initializer which allows us to injectNetworkMockup(seeOrderRemoteTests).By means of
NetworkMockup, we'll be able to specify whichjsonfile should be returned, whenever a given remote RPC is performed.Testing:
@mindgraffiti May i bug you with this one Thuy?.
cc @diegoreymendez @astralbodies We should seriously consider bringing this architecture back to WPiOS