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

Add a way to disable in all threads #717

Closed
wants to merge 1 commit into from

Conversation

jaredbeck
Copy link
Member

Expands on @Hermanverschooten's suggestion in #695, adds a way to disable PT in all threads, hopefully fixes #635.

Initially, I was concerned about having both a thread-safe flag and thread-unsafe flag. This PR mitigates that concern by giving these methods very explicit names, like enabled_in_all_threads= and enabled_in_current_thread=.

  • Renames config methods to clarify which are thread-safe and which are not. Deprecates old methods whose names were unclear.
  • Adds rails config option: config.paper_trail.enabled. See initializer in paper_trail/frameworks/rails/engine.rb
  • Organizes docs re: the many ways of disabling

Ben, I liked your suggestion in #635 that the all-threads flag be immutable, but I didn't see immediately how to implement said immutability given the way we apply the rails config option in engine.rb.

- Renames config methods to clarify which are thread-safe and
  which are not. Deprecates old methods whose names were unclear.
- Adds rails config option: config.paper_trail.enabled
  - See initializer in paper_trail/frameworks/rails/engine.rb
- Organizes docs re: the many ways of disabling

Originally contributed by Herman verschooten, heavily modified
by Jared Beck. Squashed for posterity.
@jaredbeck
Copy link
Member Author

#720 is a simpler alternative to this, based on conversation with @seanlinsley in #635

@jaredbeck
Copy link
Member Author

Closed by #723, a mutually exclusive alternative that fixes #635 but not #584.

@jaredbeck jaredbeck closed this Mar 5, 2016
@jaredbeck jaredbeck deleted the disable_in_all_threads branch July 9, 2017 02:53
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.

None yet

2 participants