#59 — Update errors #63
Conversation
|
Job #63 is now in scope, role is |
Codecov Report
@@ Coverage Diff @@
## develop #63 +/- ##
===========================================
+ Coverage 92.37% 92.94% +0.56%
===========================================
Files 59 58 -1
Lines 1509 1545 +36
Branches 26 24 -2
===========================================
+ Hits 1394 1436 +42
+ Misses 115 109 -6
Continue to review full report at Codecov.
|
| @@ -50,8 +50,8 @@ class GethNetworkTests: XCTestCase { | |||
| expect{ | |||
| try GethNetwork(ip: "127.0.0.1", port: "8545", isSecureConnection: true) | |||
There was a problem hiding this comment.
@rockfridrich
As per Contributing.md
Please put all constant values as object properties.
There was a problem hiding this comment.
This will make test less readable
There was a problem hiding this comment.
@rockfridrich I think that it is right the opposite way.
Please read this.
cc @Biboran
There was a problem hiding this comment.
Depends :)
I agree that we need to place repeated constants somewhere else. I don't know which concept can be used for this. If something is used in one particular test function only then we don't need to place it somewhere in the top of the file.
| @@ -5,7 +5,19 @@ | |||
| import CryptoSwift | |||
| import Foundation | |||
|
|
|||
| public final class InvalidAddressLengthError: Swift.Error { } | |||
| public final class IncorrectAddressLengthError: DescribedError { | |||
There was a problem hiding this comment.
@rockfridrich Please provide documentation for this class.
| @@ -11,7 +23,7 @@ public final class TransactionHashParameter: GethParameter { | |||
| } | |||
|
|
|||
| public func value() throws -> Any { | |||
| guard transactionHash.toString().count == 64 else { throw InvalidTransactionHashError() } | |||
| guard transactionHash.toString().count == 64 else { throw IncorrectTransactionHashLengthError(length: transactionHash.toString().count) } | |||
There was a problem hiding this comment.
@rockfridrich
Please limit line length to 80 chars.
We should have a swiftlint rule to enforce that (cc @Biboran )
|
@rockfridrich Please provided a proper PR description that shows what problem you have fixed an how. |
|
@Biboran @driver733 I have moved all constants to private class variables. Should we use this conversation in all tests? |
|
@rockfridrich Yes. Also, please rename all constants to the following signature, as per |
|
@driver733 uppercase makes multipal words variable unreadable. |
|
@rockfridrich Use underscore. Here's an example. |
|
@Biboran @driver733 let's adjust |
rockfridrich
force-pushed
the
feature/add_described_errors
branch
from
6cd9de0
to
08f2efa
Compare
|
Return tests back to previous state |
|
@Biboran @driver733 could we merge this? |
|
@Biboran ping |
|
@driver733/z this job was assigned to you 5 days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please. |
|
@0crat refuse |
|
@driver733 The user @driver733/z resigned from #63, please stop working. Reason for job resignation: Order was cancelled |
|
Tasks refusal is discouraged, see §6: -15 points just awarded to @driver733/z |
|
The job #63 is now out of scope |
Closes #59
This PR implements
DescribedErrorprotocol for errors in classes described in the related issue.