gdgvda / cron

a cron library for go
MIT License
13 stars 3 forks source link

feat: Prevent the creation of constant delay schedules that are not a multiple of one second. #23

Closed stubents closed 4 months ago

stubents commented 5 months ago

As this is not supported anyway by this library it might lead to confusing behaviour

closes #21

janrnc commented 5 months ago

Hi @stubents, sorry for getting back to you just now that you have already worked on this.

In my opinion ConstantDelaySchedule, as an alternative implementation of the Schedule interface, should not necessarily suffer from this one second as minimum unit problem, even though I agree with your first assumption about this use case not being very appropriate.

Considering that its logic is contained in the single line return statement of the Next() method, I think similar schedules can be implemented externally case by case. How would you feel about making both the struct and methods private? This way we could handle the error directly in the existing function, without having to deal with the unhandy usage you mentioned.

stubents commented 5 months ago

Hi @janrnc, no worries and thanks for the feedback!

I'm not quite sure if I got you right. You'd like to create another Schedule implementation that is private and only used by the parser?

janrnc commented 5 months ago

Not exactly. The idea is to make the ConstantDelaySchedule struct and Every() func private. By doing so, the way we handle the error becomes much less important because it would be just an internal component. The private every() could handle the error itself, without requiring a dedicated function as the MustEvery() you're introducing. What do you think?

stubents commented 5 months ago

Makes sense, actually I think there's no good reason not to use the @every string anyway. I'll change the PR

github-actions[bot] commented 4 months ago

:tada: This PR is included in version 0.3.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: