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

Bump up ruby version #253

Merged
merged 9 commits into from
Jun 17, 2021
Merged

Bump up ruby version #253

merged 9 commits into from
Jun 17, 2021

Conversation

ragi256
Copy link
Collaborator

@ragi256 ragi256 commented Jun 14, 2021

For #250 and #252

Upgrade the Ruby version to 3.0.0.
I want to keep the PR to only the Ruby version, but Rails didn't work with 3.0.0 as it was.
So, I had no choice but to update Rails as well.

Upgrading rails to v6 should affect the frontend(webpack) and DB migration(ActiveRecord), so we're going to use WIP to see how it goes.
As a result of minimizing upgrading v6 effects, I made very few changes.

Comment on lines -20 to -22
rows.map {|row|
raw_columns(table).zip(row).map {|column, value| adapter.type_cast(value, column) }
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

type_cast is deprecated. rails/rails#39106

These rows data are already casted.
I tried to exclude this code and then had no problem with either execution of rspec and a batch SynchronizeDataSource.

Comment on lines +5 to +10
adapter { ActiveRecord::Base.establish_connection.db_config.configuration_hash[:adapter] }
host { "localhost" }
port { 5432 }
dbname { ActiveRecord::Base.establish_connection.spec.config[:database] }
user { ActiveRecord::Base.establish_connection.spec.config[:username] }
password { ActiveRecord::Base.establish_connection.spec.config[:password] }
dbname { ActiveRecord::Base.establish_connection.db_config.configuration_hash[:database] }
user { ActiveRecord::Base.establish_connection.db_config.configuration_hash[:username] }
password { ActiveRecord::Base.establish_connection.db_config.configuration_hash[:password] }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ActiveRecored::Base.establish_connection returns ActiveRecord::ConnectionAdapters::ConnectionPool.
ConnectionPool.
ConnectionPool have a DatabaseConfig instead of ConnectionSpecification in Rails v6.1.
rails/rails#37253

Therefore, I replaced .spec.config[:symbol] to .db_config.confifuration_hash[:symbol].

gem 'html-pipeline', '~> 2.5.0'
gem 'html-pipeline-rouge_filter'
gem 'html-pipeline'
gem 'rouge'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

html-pipeline-rouge_filter has been deprecated and I will replace by rouge.
JuanitoFatas/html-pipeline-rouge_filter@322ce29

gem 'rinku'
gem 'github-markdown'
gem 'commonmarker'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

github-markdown has been deprecated and I will replace by commonmarker
https://rubygems.org/gems/github-markdown

@@ -2,7 +2,7 @@

Rails.application.config.markdown_to_html_pipeline = HTML::Pipeline.new [
HTML::Pipeline::MarkdownFilter,
HTML::Pipeline::RougeFilter,
HTML::Pipeline::SyntaxHighlightFilter,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Configuration changes due to replacement of html-pipeline-rouge_filter gem.

FROM ruby:2.6.2-slim-stretch
RUN apt-get update -qq && apt-get install -y build-essential libpq-dev default-libmysqlclient-dev nodejs-legacy curl git
FROM ruby:3.0.0-slim-buster
RUN apt-get update -qq && apt-get install -y build-essential libpq-dev default-libmysqlclient-dev nodejs libnode64 curl git
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nodejs-legacy was available up to debian streatch, but not from debian buster.
This image use nodejs and libnode64 instead.

@ragi256 ragi256 marked this pull request as ready for review June 17, 2021 04:05
@ragi256 ragi256 merged commit 147323c into cookpad:master Jun 17, 2021
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.

None yet

2 participants