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 JobIteration::Deprecation #441

Merged
merged 2 commits into from
Feb 9, 2024
Merged

Add JobIteration::Deprecation #441

merged 2 commits into from
Feb 9, 2024

Conversation

sambostock
Copy link
Contributor

This adds a Deprecation object using ActiveSupport::Deprecation, allowing us to log fancy deprecation warnings.

It has been extracted from #81, as it will also be relevant in other PRs.

I believe the missing piece #81 needed to properly work with DeprecationToolkit was that we not only needed to configure it to attach_to our deprecations, but we also needed to add our Deprecation to the host application's deprecators, so DeprecationToolkit will add the :notify behavior to it, so it has notifications to subscribe to.1 The simplest way to do this is for us to use a Railtie to register an initializer which does it.

Footnotes

  1. I feel like there's some documentation that could be improved, either in Rails or in DeprecationToolkit to capture this.

@sambostock sambostock mentioned this pull request Nov 21, 2023
2 tasks
@sambostock sambostock marked this pull request as ready for review November 23, 2023 05:43
lib/job-iteration/railtie.rb Outdated Show resolved Hide resolved
lib/job-iteration/railtie.rb Show resolved Hide resolved
lib/job-iteration/railtie.rb Outdated Show resolved Hide resolved
Copy link
Contributor Author

@sambostock sambostock left a comment

Choose a reason for hiding this comment

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

It would be nice to have tests for this that spin up a full Rails application (see #459), but in the interest of unblocking some other work, we're going to merge this as is and rely on manually testing the deprecation code paths in host apps for now.

@sambostock sambostock merged commit 1a93b9b into main Feb 9, 2024
45 checks passed
@sambostock sambostock deleted the deprecation branch February 9, 2024 17:19
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.

3 participants