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

issue #487: unify Thread.new #496

Closed
wants to merge 11 commits into from

Conversation

ColinDKelley
Copy link
Collaborator

Issue #487:

  • Unify thread creation through new Listen::Thread module.
  • Set the Thread#name = property to assist with debugging in the surrounding process.
  • rescue all exceptions in Listen::Thread and log them, including complete call stack (caller + exception backtrace) and show cause detail if any.
  • Only call one of Thread#join, Thread.kill.

spec/lib/listen/thread_spec.rb Outdated Show resolved Hide resolved
spec/lib/listen/adapter/base_spec.rb Show resolved Hide resolved
spec/acceptance/listen_spec.rb Outdated Show resolved Hide resolved
lib/listen/thread.rb Outdated Show resolved Hide resolved
lib/listen/event/loop.rb Show resolved Hide resolved
lib/listen/adapter/darwin.rb Outdated Show resolved Hide resolved
lib/listen/adapter/darwin.rb Show resolved Hide resolved
lib/listen/adapter/base.rb Show resolved Hide resolved
spec/lib/listen/thread_spec.rb Show resolved Hide resolved
spec/lib/listen/thread_spec.rb Show resolved Hide resolved
spec/lib/listen/thread_spec.rb Show resolved Hide resolved
spec/lib/listen/thread_spec.rb Show resolved Hide resolved
spec/lib/listen/thread_spec.rb Show resolved Hide resolved
spec/lib/listen/thread_spec.rb Show resolved Hide resolved
spec/lib/listen/thread_spec.rb Show resolved Hide resolved
spec/lib/listen/thread_spec.rb Show resolved Hide resolved
spec/lib/listen/thread_spec.rb Show resolved Hide resolved
spec/lib/listen/thread_spec.rb Show resolved Hide resolved
spec/lib/listen/thread_spec.rb Show resolved Hide resolved
lib/listen/thread.rb Show resolved Hide resolved
lib/listen/thread.rb Show resolved Hide resolved
lib/listen/thread.rb Show resolved Hide resolved
lib/listen/thread.rb Show resolved Hide resolved
lib/listen/thread.rb Show resolved Hide resolved
lib/listen/thread.rb Show resolved Hide resolved
lib/listen/thread.rb Show resolved Hide resolved
lib/listen/thread.rb Show resolved Hide resolved
@ColinDKelley ColinDKelley changed the title issue #487: unify thread new issue #487: unify Thread.new Sep 20, 2020
@ColinDKelley
Copy link
Collaborator Author

@jonathanhefner Can you please also review?

lib/listen/logger.rb Show resolved Hide resolved
spec/lib/listen/logger_spec.rb Show resolved Hide resolved
spec/lib/listen/logger_spec.rb Show resolved Hide resolved
spec/lib/listen/logger_spec.rb Show resolved Hide resolved
spec/lib/listen/logger_spec.rb Show resolved Hide resolved
spec/lib/listen/logger_spec.rb Show resolved Hide resolved
lib/listen/adapter/windows.rb Show resolved Hide resolved
lib/listen/adapter/windows.rb Show resolved Hide resolved
lib/listen/adapter/base.rb Show resolved Hide resolved
lib/listen/adapter.rb Show resolved Hide resolved
@ColinDKelley
Copy link
Collaborator Author

I just rebased to master.

Copy link
Member

@ioquatix ioquatix left a comment

Choose a reason for hiding this comment

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

Some minor feedback for discussion.

@ColinDKelley
Copy link
Collaborator Author

Thanks for the review, @ioquatix and @jonathanhefner. I believe all the comments are addressed, but don't hesitate to @-mention me if I missed anything, or more discussion would be helpful.

Rebasing + merging into master now.

@ColinDKelley
Copy link
Collaborator Author

Merged.

@ColinDKelley ColinDKelley deleted the issue-487/unify-thread-new branch October 30, 2020 21:37
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.

4 participants