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

Update Ruby on Rails to 7.2.0 #929

Open
wants to merge 26 commits into
base: develop
Choose a base branch
from

Conversation

KseniaGovorun
Copy link

@KseniaGovorun KseniaGovorun commented Oct 30, 2024

dev

Code reviewers

Summary of issue

Update Ruby on Rails to latest

Summary of change

Updated Ruby on Rails to 7.2.0

CHECK LIST

  • СI passed
  • Сode coverage >=95%
  • PR is reviewed manually again (to make sure you have 100% ready code)
  • All reviewers agreed to merge the PR
  • PR meets all conventions

Copy link

codecov bot commented Oct 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.73%. Comparing base (4cf826e) to head (3f4f0ea).
Report is 5 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #929      +/-   ##
===========================================
+ Coverage    91.82%   96.73%   +4.91%     
===========================================
  Files           74       73       -1     
  Lines         1052     1012      -40     
===========================================
+ Hits           966      979      +13     
+ Misses          86       33      -53     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@loqimean loqimean left a comment

Choose a reason for hiding this comment

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

зкооперуйся з Іванною Амброзяк, разом гляньте до цього ПРа, до її, аби все гарно зробити

Comment on lines 47 to 48
config.action_mailer.delivery_method = :letter_opener
config.action_mailer.perform_deliveries = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

а чому цей конфіг видалений?

Comment on lines 135 to 150
config.action_mailer.delivery_method = :smtp
config.action_mailer.raise_delivery_errors = true
config.action_mailer.perform_deliveries = true
config.action_mailer.default_url_options = {
host: "zero-waste-project.herokuapp.com",
protocol: "https"
}
config.action_mailer.smtp_settings = {
address: "smtp.sendgrid.net",
port: 587,
domain: "zero-waste-project.herokuapp.com",
user_name: ENV.fetch("SENDGRID_USERNAME", nil),
password: ENV.fetch("SENDGRID_API_KEY", nil),
authentication: "plain",
enable_starttls_auto: true
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

чому цей конфіг видалений?

# config.assume_ssl = true

# Force all access to the app over SSL, use Strict-Transport-Security, and use secure cookies.
config.force_ssl = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

а чому це додала?

Copy link
Collaborator

Choose a reason for hiding this comment

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

тут же пише, що цей файл має бути видалений. Також я хотів би, аби максимальний розмір лог файла, з цього файла, був винесений в конфігурацію

public/icon.png Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

нам цього не потрібно

Copy link
Collaborator

Choose a reason for hiding this comment

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

а нам ця міграція потрібна?

Copy link
Collaborator

Choose a reason for hiding this comment

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

також всі ці інші міграції?

@@ -3,7 +3,7 @@
require "rails_helper"

RSpec.describe UsersCsvGenerator do
let(:time_login) { Time.new(2020, 0o1, 0o1).in_time_zone("Kyiv") }
let(:time_login) { Time.new(2020, 0o1, 0o1).in_time_zone("Europe/Kyiv") }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let(:time_login) { Time.new(2020, 0o1, 0o1).in_time_zone("Europe/Kyiv") }
let(:time_login) { Time.new(2020, 0o1, 0o1) }

Copy link
Author

Choose a reason for hiding this comment

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

Rubocop не дозволяє

# Configuration for the application, engines, and railties goes here.
#
# These settings can be overridden in specific environments using the files
# in config/environments, which are processed later.
#
config.time_zone = "Kyiv"
# config.time_zone = "Central Time (US & Canada)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

де таймзона?)))

Comment on lines 25 to 28
config.i18n.available_locales = [:en, :uk]
config.i18n.default_locale = :en

config.i18n.load_path += Dir[Rails.root.join("config", "locales", "**", "*.{rb,yml}")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

а це чому видалила?

config/environments/production.rb Show resolved Hide resolved
# config.action_mailer.raise_delivery_errors = false

# Enable locale fallbacks for I18n (makes lookups for any locale fall back to
# the I18n.default_locale when a translation cannot be found).
config.i18n.fallbacks = true

# Send deprecation notices to registered listeners.
config.active_support.deprecation = :notify
Copy link
Collaborator

Choose a reason for hiding this comment

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

повертай

config/environments/test.rb Show resolved Hide resolved
config/environments/test.rb Show resolved Hide resolved
config/initializers/assets.rb Show resolved Hide resolved
config/puma.rb Show resolved Hide resolved
config/puma.rb Show resolved Hide resolved
@loqimean loqimean linked an issue Nov 9, 2024 that may be closed by this pull request
@KseniaGovorun KseniaGovorun self-assigned this Nov 11, 2024
Comment on lines 52 to 54
config.logger = ActiveSupport::Logger.new($stdout)
.tap { |logger| logger.formatter = ::Logger::Formatter.new }
.then { |logger| ActiveSupport::TaggedLogging.new(logger) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

це не гуд взагалі, глянь, як було, раніше була умова на цей конфіг

Gemfile Outdated
Comment on lines 64 to 65
gem "rubocop-factory_bot"
gem "rubocop-capybara"
Copy link
Collaborator

Choose a reason for hiding this comment

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

require false


# Show full error reports and disable caching.
config.consider_all_requests_local = true
config.action_controller.perform_caching = false
config.cache_store = :null_store

# Raise exceptions instead of rendering exception templates.
# Render exception templates for rescuable exceptions and raise for other exceptions.
config.action_dispatch.show_exceptions = :none
Copy link
Collaborator

Choose a reason for hiding this comment

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

через цю штуку тести падають

@loqimean loqimean force-pushed the develop branch 4 times, most recently from 03a3caf to 7d2dc81 Compare December 2, 2024 12:04
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.

Update Ruby on Rails to latest
3 participants