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

[Feat] TriggerBuilder: TriggerBuilder's startAt Method Settings Type Extension #1291

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

Conversation

hwangjeyeon
Copy link

@hwangjeyeon hwangjeyeon commented Dec 29, 2024

This PR extends the triggerBuilder's startAt type setting. Previously, only Date types could be delivered, so the user did the type conversion, but now LocalDateTime and ZonedDateTime can also be delivered without type conversion.

Changes

  • TriggerBuilder's startAt settings can be delivered to LocalDatetTime type
  • TriggerBuilder's startAt settings can be delivered to ZonedDateTime type

Checklist

  • tested locally
  • updated the docs
  • added appropriate test
  • signed-off on the DCO referenced in the CONTRIBUTING link below via git commit -s on my commits, and submit this code under terms of the Apache 2.0 license and assign copyright to the Quartz project owners

- Can also be set to LocalDateTime or ZonedDateTime type without conversion to Date type

Signed-off-by: hwangjeyeon <[email protected]>
- First Test, Is the time in the format LocalDateTime applied to the trigger well
- Second Test, After the start time, make sure the trigger is working well (Decided as last FireTime)

Signed-off-by: hwangjeyeon <[email protected]>
@mjeffrey
Copy link

I don't understand the use case for LocalDateTime. LocalDateTime has very few real world use cases and is typically used incorrectly. ZonedDateTime is also somewhat misunderstood.
I think it would be more useful to use Instant as a parameter instead of the other two.

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

Successfully merging this pull request may close these issues.

2 participants