[#27] [Chore] Replace Alamofire with Moya #28
Conversation
[Chore] Release first minor version 0.1.0
Shayokh144
force-pushed
the
chore/27-replace-alamofire-with-moya
branch
from
e8406ab to
eb20852
Compare
Shayokh144
force-pushed
the
chore/27-replace-alamofire-with-moya
branch
from
eb20852 to
516b39f
Compare
|
|
||
| var parameters: Parameters? { get } | ||
| enum RequestConfiguration { | ||
| case weather(cityName: String) |
There was a problem hiding this comment.
Please include line break after enum {
|
|
||
| var encoding: ParameterEncoding { get } | ||
| extension RequestConfiguration: TargetType { |
There was a problem hiding this comment.
Is this the on API? If we will have more endpoints I suggest creating a new file for each endpoint. TargetType+Weather.swift
There was a problem hiding this comment.
- After creating new file should I rename the
enum RequestConfigurationtoenum WeatherRequestConfiguration? - By
I suggest creating a new file for each endpointdid you mean creating new file for eachTarget? like oneTargetcan contain multiple cases with different paths/endpoints like here but oneTargetis for onebaseUrl?
There was a problem hiding this comment.
- I would say that it's easier to declare a new enum for each sub-service (similar APIs) because how
enumin Swift works. - One Target enum can have multiple endpoints, divided by the business use case OR by the endpoints.
If we will only have one/two endpoints in this project then the refactor is not needed.
|
|
||
| var headers: HTTPHeaders? { nil } | ||
| // Optional Stub |
There was a problem hiding this comment.
Please provide more descriptive comment, what is Optional Stub, do we have to do something about it in the future: TODO:.
I don't think we have to include sampleData in this declaration, correct me if I'm wrong.
There was a problem hiding this comment.
you are right.. this sampleData is for creating mock response, will add details comment.
There was a problem hiding this comment.
Same with comment with documentation.
According to the source code , this property is optional, so we don't have to declare it when using the default value.
|
|
||
| var interceptor: RequestInterceptor? { nil } | ||
| // Optional Stub |
There was a problem hiding this comment.
This is not optional since we declare some real value to it, I suggest removing this comment.
There was a problem hiding this comment.
By the comment I want to say that : these stubs can be skipped, actually I try to pretend like this is for iOS Template.
There was a problem hiding this comment.
I don't think we need to add comment as a documentation. When we use a library, the document is available on the library's repository. Adding library's comment in the project will lead to: - bloated code base & - need to revise comment if the library document changes.
We assume reader of this code knows Moya (because we pick Moya as part of the project) so we should not include the comment.
#27
What happened
Replace
Alamofirenetworking library withMoya.Insight
Alamofirefrom podMoyain podMoyaProof Of Work
Screen.Recording.2022-11-25.at.11.47.45.AM.mov
Remarks