Conversation
bitProfessor
commented
Sep 6, 2019
- DEXBot/tests/strategies/relative_orders/conftest.py
- DEXBot/tests/strategies/relative_orders/test_relative_orders.py
- DEXBot/tests/strategies/relative_orders/test_relative_orders_unittests.py
- DEXBot/tests/strategies/relative_orders/test_single_account_multiple_workers_unittests.py
If custom expiration is set and it's too small, it makes a bad condition for worker with cp_from_last_trade active. So just make sure we have default expiration when cp_from_last_trade is used.
* if worker newly created, use market center price * if worker is not new, always try to find own last trade even on startup Closes: Codaone#672
Don't limit to 1 op, get all ops and iterate to find first match. This is a preparation to run multiple workers on same account.
|
Better to merge into devel rather than master until these tests have been reviewed |
|
Ok
On 09/06/2019 19:30, PermieBTS wrote:
Better to merge into devel rather than master until these tests have been reviewed
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Why close? It's been edited to merge to devel now, vvk altered it 👍
|
|
@bitProfessor no need to open-close multiple PRs. A good workflow is:
|
|
I think I misunderstood it, thinking that I should check the code again.
On 09/07/2019 23:21, Vladimir Kamarzin wrote:
@bitProfessor no need to open-close multiple PRs. A good workflow is:
create a branch to work on a feature
do PR once the feature is ready
forgot something? Add more commits into PR
Avoid force pushes after opening a PR (exception: if you 100% know if nobody else working on it, and you really need to adjust past commits)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@joelvai when you get back from vacation can we have this merged asap please :) With the aim to release Single Account Multiple Workers soon after when tests confirm the feature is satisfactory |
|
@bitProfessor make necessary fixes so that the Travis build passes and there appears to be two functions with same name |
|
ok
| |
bitprofessor
邮箱：bitprofessor@163.com
|
Signature is customized by Netease Mail Master
On 09/27/2019 20:52, joelvai wrote:
@bitProfessor make ne nessesary fixes so that the Travis build passes and there appears to be two functions with same name test_correct_asset_names.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
@joelvai I would like to review/improve these tests before merging. |
|
@bitfag are you able to review these tests by tomorrow evening? Or ASAP to be ready in good time for @joelvai to merge on friday? |
|
I'm starting to work on it. I'll post time estimations later on. |
|
These tests are partially copypaste from KOTH tests, but RO strategy requires testing of different things. Overall coverage is too surfaced, more deep coverage is needed. Huge refactor needed. At least several workdays are needed. |
|
Hmm ok thanks @bitfag If you don't finish them in time for @joelvai to merge on friday we can re-assess |
We're not using random.xxx() for security purposes.
|
@joelvai this is ready for review, I also merged here my ro-refactor branch because it contains the fixes needed to run RO tests. |