Skip to content
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

[shell-operator] feat/new operator event handling mechanism #735

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ldmonster
Copy link
Contributor

@ldmonster ldmonster commented Mar 17, 2025

Overview

Replace functional pattern with return values

What this PR does / why we need it

Special notes for your reviewer

Pavel Okhlopkov added 5 commits March 13, 2025 18:27
Signed-off-by: Pavel Okhlopkov <[email protected]>
Signed-off-by: Pavel Okhlopkov <[email protected]>
Signed-off-by: Pavel Okhlopkov <[email protected]>
Signed-off-by: Pavel Okhlopkov <[email protected]>
@ldmonster ldmonster changed the title Feat/new operator event handling mechanism [shell-operator] feat/new operator event handling mechanism Mar 17, 2025
@ldmonster ldmonster requested review from yalosev and miklezzzz March 17, 2025 12:18
@ldmonster ldmonster self-assigned this Mar 17, 2025
@ldmonster ldmonster requested a review from juev March 17, 2025 12:19
@ldmonster ldmonster added the enhancement New feature or request label Mar 17, 2025
@ldmonster ldmonster marked this pull request as ready for review March 18, 2025 06:40
}

infos := hc.ScheduleController.HandleEvent(crontab)
if createTasksFn == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And why do we check the input parameters only after we have called HandleEvent? If the functions have not been transferred, we do not use the result that we received earlier.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like untouched code. this is not my logic.
previously, we have no returned values, just append to slice in closure.
now we have more clear structure with returning tasks values, if we handle it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just pointed out what the eye caught on, the correction seems to be small.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants