-
Notifications
You must be signed in to change notification settings - Fork 284
Conversation
11e053d
to
cadc186
Compare
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.
LGTM!
One nitty comment and will approve when the threading question is solved
@@ -1163,6 +1182,30 @@ def test_already_assigned( | |||
dispatcher_mock.send.assert_not_called() | |||
logger_mock.error.assert_called() | |||
|
|||
def test_task_api( | |||
self, _logger_mock, _dispatcher_mock, |
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.
one argument per line from the style guide
@maaktweluit I consider the threading issue resolved. |
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.
ok, thanks!
Codecov Report
@@ Coverage Diff @@
## develop #4561 +/- ##
===========================================
- Coverage 90.33% 90.33% -0.01%
===========================================
Files 225 225
Lines 19929 19944 +15
===========================================
+ Hits 18003 18016 +13
- Misses 1926 1928 +2 |
@Krigpl Please create a follow-up PR solving the |
@mfranciszkiewicz Didn't you find that this runs in a separate thread anyway so it doesn't effectively matter much anyway at the moment? |
@Krigpl ATM it runs in a separate thread through |
Yet another two if conditions.