-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
check if it is running when calling start() #54
Conversation
@@ -123,6 +123,9 @@ func (c *Cron) Entries() []*Entry { | |||
|
|||
// Start the cron scheduler in its own go-routine. | |||
func (c *Cron) Start() { | |||
if c.running { | |||
return | |||
} |
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.
The method documentation says:
// Start the cron scheduler in its own go-routine.
But it's not what this code does, so docs would need to be updated if this change is to be merged.
See #46 which was an identical issue. The conclusion seemed to be that it's better for your program to arrange it so that it doesn't call |
@shurcooL I think this nessesory to check running here. As |
@@ -121,8 +121,11 @@ func (c *Cron) Entries() []*Entry { | |||
return c.entrySnapshot() | |||
} | |||
|
|||
// Start the cron scheduler in its own go-routine. | |||
// Start the cron scheduler in its own go-routine if it is not running, otherwize it does nothing. |
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.
// Start the cron scheduler in its own go-routine, or no-op if already started.
OK, I guess this comes up enough and it's easy to resolve that might as well.. small tweak to comment please! |
Thank you! |
fix a bug,
call Start() more than once will start multiple run() goroutines to run the jobs.