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

isOverdue documentation in Scheduler is confusing #25223

Merged
merged 1 commit into from
May 24, 2022

Conversation

rmanibus
Copy link
Contributor

No description provided.

@quarkus-bot
Copy link

quarkus-bot bot commented Apr 28, 2022

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should preferably start with an uppercase character (if it makes sense!)

This message is automatically generated by a bot.

@rmanibus
Copy link
Contributor Author

@mkouba

@@ -31,7 +31,7 @@ public interface Trigger {
/**
* The grace period is configurable with {@link Scheduled#overdueGracePeriod()}.
* <p>
* This method returns {@code false} if the last execution has been skipped.
* This method returns {@code false} if the last execution should have been fired but was not.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should mention that is wa not fired within the fire time + grace period... I'm not sure about the wording.

Copy link
Member

Choose a reason for hiding this comment

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

Any chance we could finalize this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@rmanibus ^ ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hi @mkouba @gsmet, I reworded it, let me now if it's ok for you !

Copy link
Contributor Author

@rmanibus rmanibus May 21, 2022

Choose a reason for hiding this comment

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

@mkouba I'm reading this again, and what I wanted to express initially in this message was that If a scheduler execution is skipped, it will not be considered as overdue.

Copy link
Contributor Author

@rmanibus rmanibus May 21, 2022

Choose a reason for hiding this comment

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

I think the following is better:

    /**
     * The grace period is configurable with {@link Scheduled#overdueGracePeriod()}.
     * <p>
     * Skipped executions are not considered as overdue.
     *
     * @return {@code false} if the last execution took place between
     * the expected execution time and the end of the grace period, {@code true} otherwise
     * @see Scheduled#overdueGracePeriod()
     */

@quarkus-bot

This comment has been minimized.

@rmanibus rmanibus force-pushed the trigger_doc branch 3 times, most recently from ad48918 to 59c58be Compare May 21, 2022 14:25
@quarkus-bot

This comment has been minimized.

@mkouba mkouba merged commit 0073fc2 into quarkusio:main May 24, 2022
@quarkus-bot quarkus-bot bot added this to the 2.10 - main milestone May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants