MINOR: rename TaskRegistry methods to better reflect their purpose.#21448
MINOR: rename TaskRegistry methods to better reflect their purpose.#21448Nikita-Shupletsov wants to merge 1 commit intoapache:trunkfrom
Conversation
| @Override | ||
| public synchronized void clear() { | ||
| pendingTasksToInit.clear(); | ||
| pendingTasksToClose.clear(); |
There was a problem hiding this comment.
This seems to be the only change not related to re-naming; what's the impact of this change? Sound like a bug, but maybe only minor?
There was a problem hiding this comment.
Yeah, I missed that part:(
the only usage of this list is to verify that we are deleting an existing task, so even though it's technically a bug, I can't think of an impact of it, as it's not used anywhere outside of shutdown and only for validation purposes
|
Overall LGTM -- I will hold off merging before AK 4.2.0 got release, because I would like to cherry-pick it to 4.2 branch (main purpose is to keep future cherry-pick to 4.2 clean). Open to also cherry-pick to 4.1, but it seems more unlikely that there would be another 4.1.3 bug-fix release, so maybe not worth. -- I would for sure "exclude" 4.0 branch as "too old". Thoughts? |
makes sense to me |
|
LGTM, thanks for following up to clean this up! |
Changed the name of method that work only with initialized tasks(not
pending) to better reflect their purpose.
Reviewers: Matthias J. Sax matthias@confluent.io, Lucas Brutschy
lbrutschy@confluent.io