-
Notifications
You must be signed in to change notification settings - Fork 340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove empty callbacks #1488
base: master
Are you sure you want to change the base?
Remove empty callbacks #1488
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1488 +/- ##
==========================================
+ Coverage 83.86% 83.87% +0.01%
==========================================
Files 122 122
Lines 11148 11140 -8
Branches 948 946 -2
==========================================
- Hits 9349 9344 -5
+ Misses 1486 1485 -1
+ Partials 313 311 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
These are virtual methods defined in the LifecycleNodeInterface, and don't need to be overridden if there is no action to be done at the respective transition. Why they are here? I guess just because of copying a template around. |
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.
Thanks for the cleanup.
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
Follow up of #1477 (review). This is to remove empty callback, e.g.
on_cleanup
,on_deactivate
from code base.A noob question, based on node life_cycle doc , there are predefined roles for these call backs. Why were these coded as empty ? Was it for readability or a place holder for further development ?