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

Use ActiveSupport::Deprecator's API #113

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

olleolleolle
Copy link
Contributor

@olleolleolle olleolleolle commented Oct 29, 2024

This PR fixes a wrong usage of Rails' Deprecator API.

In addition:

  • it drops a few EOL Ruby versions from the CI matrix
  • it drops a few EOL Rails version from the CI matrix

Gets to green.

else
warn(msg)
end
end

def self.deprecator
Copy link
Contributor

Choose a reason for hiding this comment

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

It's hilarious that they've deprecated their deprecator.

Is there any way we can support both the new and old styles together and keep old Rails support going? I'm ok to drop old versions if it becomes really onerous, but I wonder if there's a simple fix that keeps everything working.

I think this is the commit where things changed...
rails/rails@c682bf2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, we can work around the Rails API detail with conditions.

But, the harder problem was that setup-ruby failed to install gems on a more basic level, when the bundler: 1 was configured.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have the time I wish I had to troubleshoot CI.

I'm ok to drop the old CI matrix versions for now just to get this out. Would you be able to add the compatibility conditional to make sure we still work on older Rails? Then I'll get this merged and out the door.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😅 It may take some time, but we'll get there.

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