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

Makes Tailer use ScheduledExecutorService from Java 5 #2

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

triceo
Copy link

@triceo triceo commented May 17, 2014

This has multiple benefits:

  • it provides periodic runs of the read operations without needing to
    invoke Thread.sleep().
  • it removes any looping from the run() method, making for cleaner code.
  • generally brings Tailer closer to what modern Java looks like.

No public API changes have been made. All tests are passing without
modifications.

This has multiple benefits:
- it provides periodic runs of the read operations without needing to
invoke Thread.sleep().
- it removes any looping from the run() method, making for cleaner code.
- generally brings Tailer closer to what modern Java looks like.

No public API changes have been made. All tests are passing without
modifications.
@garydgregory
Copy link
Member

@triceo
May you rebase on master?

@garydgregory
Copy link
Member

garydgregory commented Sep 23, 2021

@triceo
May you plerase rebase on master? The implementation has changed to use the builder pattern with an internal Duration instead of long to track the sleep delay. It looks like the code needs to be adapted to configure the builder with an executor.

@triceo
Copy link
Author

triceo commented Sep 23, 2021

@garydgregory I may eventually get to it, but considering that the original PR is from 2014, I have moved on.

@garydgregory
Copy link
Member

Either it's useful or not, regardless of your moving about 😉

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