Optimise test framework (bunq/sdk_java#78) #85
Conversation
|
@patrickdw1991 please 👀 |
| private static final String LANGUAGE_EN_US = "en_US"; | ||
| private static final String REGION_NL_NL = "nl_NL"; | ||
| private static final String GEOLOCATION_ZERO = "0 0 0 0 000"; | ||
| public static final String LANGUAGE_EN_US = "en_US"; |
There was a problem hiding this comment.
hmm., i normally group constants in groups, not on visibility. Error constants on top, ten the first used group in the class. So there could indeed be a mixture of visibility in a specific group.
| private static final String FIELD_API_KEY = "ApiKey"; | ||
| private static final int INDEX_FIRST = 0; | ||
|
|
||
| protected static final String TEST_CONFIG_PATH = "bunq-test.conf"; |
| requestSpendingMoney(); | ||
|
|
||
| try { | ||
| Thread.sleep(500); |
There was a problem hiding this comment.
To ensure the spending money request has been accepted before refreshing. The refresh part will be added with #79
| secondMonetaryAccountBank = MonetaryAccountBank.get(response.getValue()).getValue(); | ||
| } | ||
|
|
||
| private static void requestSpendingMoney() { |
There was a problem hiding this comment.
People using the SDK might not really understand what's actually happening here, maybe just add a comment.
There was a problem hiding this comment.
but, this is internal lib ? User shouldn't even know whats is happening here ? 🤔
There was a problem hiding this comment.
It's open source. People might want to look into why a request was randomly created and paid.
There was a problem hiding this comment.
in the tests ? :P i have this rule, if i need to write a doc block explaining something then the method name is not good enough or should be split in multiple methods. is this the case here ? 🤔
There was a problem hiding this comment.
In the test. We actually mention how to run the tests in the README.md.
In this case the method name is clear but the process is quite weird (the sugar daddy stuff), I'm also not a big fan of comments but somethimes stuff happens outside of the code and it's not clear that's where you want to explain a bit.
This is open source code so people will actually look at it to understand, it's not something internal where we just point to a wiki. Or you can just create a wiki for the github page :P
There was a problem hiding this comment.
Will write a comment in then 👍
| } | ||
|
|
||
| } | ||
| protected static Pointer getPointerBravo() { |
There was a problem hiding this comment.
And wasn't the convention to always add (empty) docblocks?
There was a problem hiding this comment.
i thought we agreed to there is no need for it anymore 🤔 as it didn't bring any value as its a strict typed lang.
There was a problem hiding this comment.
We agreed on being consistent I think. I have posted multiple times that they where not there and you added it multiple times as that was the way you did it before. Switching styles all the time is messy, do it or don't but make sure to do it consistent.
There was a problem hiding this comment.
Alrighty then, as its a strict lang, no need for empty doc blocks. in the non strict ones will add doc blocks to define parameters etc.
There was a problem hiding this comment.
Ok so you're gonna change this in the generator?
There was a problem hiding this comment.
yea, but in a future PR then. That might cost some time. Will create followup issue.
References #78