[CM] Better readability and maintainability: rename variables, move code to more logical places 🔧 #1227
Conversation
destogl
commented
Dec 18, 2023
destogl
commented
- Move 'manage_switch' method and 'do_switch' flag to more appropriate positions.
- Make variable name clearer: rename 'request' to 'controller_name'.
github-actions
bot
requested review from
arne48,
bmagyar,
DasRoteSkelett,
olivier-stasse,
peterdavidfagan and
VanshGehlot
bmagyar
changed the title
| } | ||
|
|
||
| if (controller->is_async()) | ||
| { | ||
| async_controller_threads_.at(controller_name)->activate(); | ||
| } | ||
| } | ||
| // All controllers activated, switching done | ||
| switch_params_.do_switch = false; |
There was a problem hiding this comment.
this changes behaviour, nicht wahr?
I see it was moved into manage_switch() to the end
There was a problem hiding this comment.
No, it should not change behavior, but just put it in a more logical place.
There was a problem hiding this comment.
Actually here it might be wrong. Because the "switching" is not (theoretically( finished here if one add something more to the mange_switch method.
There was a problem hiding this comment.
I think what you said makes sense. It should be at the end of the manage_switch method.
| } | ||
|
|
||
| if (controller->is_async()) | ||
| { | ||
| async_controller_threads_.at(controller_name)->activate(); | ||
| } | ||
| } | ||
| // All controllers activated, switching done | ||
| switch_params_.do_switch = false; |
There was a problem hiding this comment.
I think what you said makes sense. It should be at the end of the manage_switch method.
|
Actually the CI is failing. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1227 +/- ##
==========================================
- Coverage 48.02% 47.83% -0.19%
==========================================
Files 41 41
Lines 3525 3472 -53
Branches 1912 1887 -25
==========================================
- Hits 1693 1661 -32
- Misses 442 446 +4
+ Partials 1390 1365 -25
Flags with carried forward coverage won't be shown. Click here to find out more.
|
destogl
force-pushed
the
optimize-cm-code
branch
from
9a1e13a
to
771f4ab
Compare
