Skip to content
This repository has been archived by the owner on Dec 13, 2023. It is now read-only.

Update fastly-rails #86

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

Update fastly-rails #86

wants to merge 4 commits into from

Conversation

tyrauber
Copy link

fastly-rails hasn’t been updated in over 2 years. With the release of factly-ruby 2.3.0, that adds thread safety and concurrency, this gem needs to be updated as well.

Changes:

  • update factly-ruby to 2.3.0
  • factory_girl_rails is deprecated, migrate to factory_bot_rails
  • remove old appraisals (deprecated ruby and rails versions and conflicts with bundler update to 2.0)
  • test ruby 2.6.3 and rails 5.2.3
  • fix test failures (get :show, params: {})
  • fix minitest deprecations (asset_nil)
  • Rails 5 before_action

fastly-rails hasn’t been updated in over 2 years. With the release of factly-ruby 2.3.0, that adds thread safety and concurrency, this gem needs to be updated as well.

Changes:

- update factly-ruby to 2.3.0
- factory_girl_rails is deprecated, migrate to factory_bot_rails
- remove old appraisals (deprecated ruby and rails versions and conflicts with bundler update to 2.0)
- test ruby 2.6.3 and rails 5.2.3
- fix test failures (get :show, params: {})
- fix minitest deprecations (asset_nil)
- Rails 5 before_action
@tyrauber
Copy link
Author

We've been running this branch in production for the last two weeks, with a million+ uniques and 100K+ purges, without a single thread concurrency error. This is ready to merge and addresses #79, #82 and #84. Thanks a ton, @thommahoney, you're awesome.

@thommahoney
Copy link
Member

@tyrauber thanks for opening this PR. Do you know where rails publishes their actively supported versions? I found https://guides.rubyonrails.org/maintenance_policy.html

Copy link
Member

@thommahoney thommahoney left a comment

Choose a reason for hiding this comment

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

if you are able, could you address the two comments that I left? thank you again. 🙌

test/fastly-rails_test.rb Outdated Show resolved Hide resolved
@tyrauber
Copy link
Author

Do you know where rails publishes their actively supported versions? I found https://guides.rubyonrails.org/maintenance_policy.html

That's the closest you will find. You can also look at the rubygem releases to see frequency of releases. There was a security update pushed for 4.2.11.1 in March. No one should be running anything less than that on production, and that is questionable, IMHO.

We could/should probably update the appraisals to drop support for 3.2.18, 4.0.5, and update 4.1.1 to 4.2.11.1 and 5.0 to 5.2.3.

- Ruby 2.6.3, 2.5.5, 2.4.6
- Rails 4.2.11.1, 5.2.3
- fix assert_equal in fastly-rails.test for user and password
…bundler 2.0 and < Rails 5 require < bundler 2.0, and > Rails 5 requires bundler 2.0.
@tyrauber
Copy link
Author

@thommahoney I updated travis to support Ruby 2.6.3, 2.5.5, and 2.4.6 Unfortunately, due to a incompatibility in bundler versions and rails versions, I was unable to figure out how to configure appraisal to support both Rails 5.2.3 and 4.2.11.1, as both require different versions of bundler.

@satishkunisi
Copy link

This is awesome 🎉 I'm excited to finally get those thread errors resolved. Just a gentle bump to see if we can get this merged and released. Thank you @tyrauber @thommahoney !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants