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

Upgrade to Rails 7 #5058

Merged
merged 44 commits into from
Feb 17, 2023
Merged

Upgrade to Rails 7 #5058

merged 44 commits into from
Feb 17, 2023

Conversation

cjcolvar
Copy link
Member

@cjcolvar cjcolvar commented Feb 7, 2023

Builds on top of ruby 3 branch

@cjcolvar cjcolvar force-pushed the rails7 branch 3 times, most recently from 21aa3cd to 46e03ec Compare February 13, 2023 15:43
Gemfile Outdated
gem 'active-fedora', '~> 13.2', '>= 13.2.5'
gem 'active_fedora-datastreams', '~> 0.4'
#gem 'active-fedora', '~> 14.0'
gem 'active-fedora', git: "https://github.com/samvera/active_fedora.git", branch: "errors_fix"
Copy link
Member Author

Choose a reason for hiding this comment

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

Temporarily use branch until PR is merged and release is cut. This PR doesn't need to wait on this though since we can clean that up in a future PR before next avalon release.

@@ -53,7 +56,8 @@ gem 'iiif_manifest', '~> 0.6'
gem 'rack-cors', require: 'rack/cors'
gem 'rails_same_site_cookie'
gem 'recaptcha', require: 'recaptcha/rails'
gem 'samvera-persona', '~> 0.3'
#gem 'samvera-persona', '~> 0.3'
gem 'samvera-persona', git: "https://github.com/samvera-labs/samvera-persona.git", branch: "newer_rails"
Copy link
Member Author

Choose a reason for hiding this comment

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

Temporarily use branch until PR is merged and release is cut. This PR doesn't need to wait on this though since we can clean that up in a future PR before next avalon release.

gem 'noid-rails', '~> 3.0.1'
gem 'ldp', '~> 1.1.0'
gem 'noid-rails', '~> 3.1'
gem 'om', git: 'https://github.com/avalonmediasystem/om.git', branch: 'ruby3'
Copy link
Member Author

Choose a reason for hiding this comment

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

Use a fork because upstream is in samvera-deprecated. We should consider contributing this back up and making a new release of om if we can figure out the process.

@@ -75,13 +79,13 @@ gem 'audio_waveform-ruby', '~> 1.0.7', require: 'audio_waveform'
gem 'browse-everything', git: "https://github.com/avalonmediasystem/browse-everything.git", branch: 'v1.2-avalon'
gem 'fastimage'
gem 'media_element_add_to_playlist', git: 'https://github.com/avalonmediasystem/media-element-add-to-playlist.git', tag: 'avalon-r6.5'
gem 'mediainfo', git: "https://github.com/avalonmediasystem/mediainfo.git", branch: 'avalon_fixes'
gem 'mediainfo', git: "https://github.com/avalonmediasystem/mediainfo.git", branch: 'rails6.1'
Copy link
Member Author

Choose a reason for hiding this comment

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

We should explore upgrading and moving off of our fork, but for now this doesn't change what we've been doing.

Comment on lines 96 to +99
gem 'sidekiq', '~> 6.2'
gem 'sidekiq-cron', '~> 1.2', git: "https://github.com/avalonmediasystem/sidekiq-cron.git", tag: 'v1.2.1-avalon'
gem 'sidekiq-cron', '~> 1.9'
Copy link
Member Author

Choose a reason for hiding this comment

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

Sidekiq has a 7.x release that we could consider upgrading to.

@@ -5,7 +5,7 @@ jobs:
build:
docker:
# Primary container image where all steps run.
- image: avalonmediasystem/avalon:7.3.0-dev
- image: avalonmediasystem/avalon:7.5.0-dev-ruby3
Copy link
Member Author

Choose a reason for hiding this comment

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

This will be changed in a follow up PR after this is merged.

@cjcolvar cjcolvar marked this pull request as ready for review February 14, 2023 18:10
This was referenced Feb 17, 2023
Copy link
Contributor

@masaball masaball left a comment

Choose a reason for hiding this comment

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

Looks good. Does rails 7 change the behavior of single vs double quotes? Curious about that being a decent amount of the changes.

@cjcolvar
Copy link
Member Author

When I ran the app:update task I reviewed the differences between what we had and what the defaults are in Rails 7 and went with the defaults if it made sense. Most of those changes were the single->double quotes.

@cjcolvar cjcolvar merged commit d827514 into develop Feb 17, 2023
@cjcolvar cjcolvar deleted the rails7 branch February 17, 2023 18:16
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