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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 3 additions & 61 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,50 +14,10 @@ jobs:

strategy:
matrix:
ruby: ["2.6", "2.7", "3.0", "3.1", "3.2", "3.3", "jruby-head"]
gemfile: ["4.2", "5.0", "5.1", "5.2", "6.0", "6.1", "7.0", "7.1"]
ruby: ["3.0", "3.1", "3.2", "3.3", "jruby-head"]
gemfile: ["6.0", "6.1", "7.0", "7.1"]

exclude:
- gemfile: "4.2"
ruby: "3.0"
- gemfile: "4.2"
ruby: "3.1"
- gemfile: "4.2"
ruby: "3.2"
- gemfile: "4.2"
ruby: "3.3"
- gemfile: "4.2"
ruby: "jruby-head"
- gemfile: "5.0"
ruby: "3.0"
- gemfile: "5.0"
ruby: "3.1"
- gemfile: "5.0"
ruby: "3.2"
- gemfile: "5.0"
ruby: "3.3"
- gemfile: "5.0"
ruby: "jruby-head"
- gemfile: "5.1"
ruby: "3.0"
- gemfile: "5.1"
ruby: "3.1"
- gemfile: "5.1"
ruby: "3.2"
- gemfile: "5.1"
ruby: "3.3"
- gemfile: "5.1"
ruby: "jruby-head"
- gemfile: "5.2"
ruby: "3.0"
- gemfile: "5.2"
ruby: "3.1"
- gemfile: "5.2"
ruby: "3.2"
- gemfile: "5.2"
ruby: "3.3"
- gemfile: "5.2"
ruby: "jruby-head"
- gemfile: "6.0"
ruby: "3.2"
- gemfile: "6.0"
Expand All @@ -66,19 +26,6 @@ jobs:
ruby: "3.2"
- gemfile: "6.1"
ruby: "3.3"
- gemfile: "7.0"
ruby: "2.5"
- gemfile: "7.0"
ruby: "2.6"
- gemfile: "7.0"
ruby: "2.7"
- gemfile: "7.1"
ruby: "2.5"
- gemfile: "7.1"
ruby: "2.6"
- gemfile: "7.1"
ruby: "2.7"


env:
BUNDLE_GEMFILE: gemfiles/rails_${{ matrix.gemfile }}.gemfile
Expand All @@ -87,13 +34,9 @@ jobs:
steps:
- uses: actions/checkout@v4

- name: "Install Ruby ${{ matrix.ruby }}"
# To automatically get bug fixes and new Ruby versions for ruby/setup-ruby,
# change this to (see https://github.com/ruby/setup-ruby#versioning):
# uses: ruby/setup-ruby@v1
- name: "Install Ruby ${{ matrix.ruby }}"
uses: ruby/setup-ruby@v1
with:
bundler: 1
ruby-version: ${{ matrix.ruby }}
bundler-cache: true # runs 'bundle install' and caches installed gems automatically

Expand All @@ -102,4 +45,3 @@ jobs:

- name: Run standard.rb
run: bundle exec rake standard
if: ${{ ! startsWith(matrix.ruby, '2.') }}
8 changes: 0 additions & 8 deletions gemfiles/rails_4.2.gemfile

This file was deleted.

8 changes: 0 additions & 8 deletions gemfiles/rails_5.0.gemfile

This file was deleted.

8 changes: 0 additions & 8 deletions gemfiles/rails_5.1.gemfile

This file was deleted.

8 changes: 0 additions & 8 deletions gemfiles/rails_5.2.gemfile

This file was deleted.

6 changes: 5 additions & 1 deletion lib/validates_email_format_of.rb
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,15 @@ def self.validate_domain_part_syntax(domain, idn: true)

def self.deprecation_warn(msg)
if defined?(ActiveSupport::Deprecation)
ActiveSupport::Deprecation.warn(msg)
deprecator.warn(msg)
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.

@deprecator ||= ActiveSupport::Deprecation.new("2.0", "validates_email_format_of")
end
end

require "validates_email_format_of/active_model" if defined?(::ActiveModel) && !(ActiveModel::VERSION::MAJOR < 2 || (ActiveModel::VERSION::MAJOR == 2 && ActiveModel::VERSION::MINOR < 1))
Expand Down