-
Notifications
You must be signed in to change notification settings - Fork 444
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
Split library into modules #589
Split library into modules #589
Conversation
It's building (but not testing yet)
Sources/Core/EthereumNetwork/Request/APIRequest+UtilityTypes.swift
Outdated
Show resolved
Hide resolved
Sources/Core/EthereumNetwork/RequestParameter/APIRequestParameterType.swift
Show resolved
Hide resolved
Sources/Core/EthereumNetwork/RequestParameter/APIRequestParameterType.swift
Outdated
Show resolved
Hide resolved
@yaroslavyaroslav I'll probably throw some additional comments into this PR. 🙂 |
extension APIRequest { | ||
var method: REST { | ||
switch self { | ||
default: return .POST |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a weird computed variable.
Is it correct that it always returns .POST
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually yes, since none of Eth API uses any other REST method. But i'd either think that it's redundant, and could be just dropped in a favor of hardcoding this method in URLSession configuration.
Sources/Core/EthereumNetwork/Request/APIRequest+ComputedProperties.swift
Outdated
Show resolved
Hide resolved
I didn't want to leave more comments like this as there are a lot of ERC/EIP standards in the project. We have two options here:
|
@JeneaVranceanu thank you for your feedback, it's really helpful, i'll dig it deeply in few days ahead. |
Delete redundant `APIRequestParameterElementType` protocol.
Dropping redundant `RequestParameter`.
Add Web3Error.clientError(code: Int)
I'd prefer second one, because I'm planning to dedicate one or two last RC to improving documentation. |
Opened an issue for documentation - #592 |
@JeneaVranceanu please take a look at the PR updates that I've made up to your issues. All that've marked with thumb up emoji should be done. And if so mark them as resolved one please. |
Great! Thanks for taking time to update things. |
No description provided.