Fix warnings in Ruby 2.7 and Ruby 3.0 #5070
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| files[:comments_summary] << proposals_comments_summary_filename if File.exists?(data_folder.join(proposals_comments_summary_filename)) | ||
| files[:comments_summary] << investments_comments_summary_filename if File.exists?(data_folder.join(investments_comments_summary_filename)) | ||
| files[:tags] << proposals_tags_filename if File.exist?(data_folder.join(proposals_tags_filename)) | ||
| files[:tags] << proposals_taggings_filename if File.exist?(data_folder.join(proposals_taggings_filename)) |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [111/110] (https://rubystyle.guide#max-line-length)
| files[:tags] << proposals_tags_filename if File.exist?(data_folder.join(proposals_tags_filename)) | ||
| files[:tags] << proposals_taggings_filename if File.exist?(data_folder.join(proposals_taggings_filename)) | ||
| files[:tags] << investments_tags_filename if File.exist?(data_folder.join(investments_tags_filename)) | ||
| files[:tags] << investments_taggings_filename if File.exist?(data_folder.join(investments_taggings_filename)) |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [115/110] (https://rubystyle.guide#max-line-length)
| files[:tags] << proposals_taggings_filename if File.exist?(data_folder.join(proposals_taggings_filename)) | ||
| files[:tags] << investments_tags_filename if File.exist?(data_folder.join(investments_tags_filename)) | ||
| files[:tags] << investments_taggings_filename if File.exist?(data_folder.join(investments_taggings_filename)) | ||
| files[:related_content] << proposals_related_filename if File.exist?(data_folder.join(proposals_related_filename)) |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [120/110] (https://rubystyle.guide#max-line-length)
| files[:tags] << investments_tags_filename if File.exist?(data_folder.join(investments_tags_filename)) | ||
| files[:tags] << investments_taggings_filename if File.exist?(data_folder.join(investments_taggings_filename)) | ||
| files[:related_content] << proposals_related_filename if File.exist?(data_folder.join(proposals_related_filename)) | ||
| files[:related_content] << investments_related_filename if File.exist?(data_folder.join(investments_related_filename)) |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [124/110] (https://rubystyle.guide#max-line-length)
| files[:tags] << investments_taggings_filename if File.exist?(data_folder.join(investments_taggings_filename)) | ||
| files[:related_content] << proposals_related_filename if File.exist?(data_folder.join(proposals_related_filename)) | ||
| files[:related_content] << investments_related_filename if File.exist?(data_folder.join(investments_related_filename)) | ||
| files[:comments_summary] << proposals_comments_summary_filename if File.exist?(data_folder.join(proposals_comments_summary_filename)) |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [139/110] (https://rubystyle.guide#max-line-length)
| files[:related_content] << proposals_related_filename if File.exist?(data_folder.join(proposals_related_filename)) | ||
| files[:related_content] << investments_related_filename if File.exist?(data_folder.join(investments_related_filename)) | ||
| files[:comments_summary] << proposals_comments_summary_filename if File.exist?(data_folder.join(proposals_comments_summary_filename)) | ||
| files[:comments_summary] << investments_comments_summary_filename if File.exist?(data_folder.join(investments_comments_summary_filename)) |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [143/110] (https://rubystyle.guide#max-line-length)
| @@ -1,4 +1,4 @@ | |||
| shared_examples "mappable" do |mappable_factory_name, mappable_association_name, mappable_new_path, mappable_edit_path, mappable_show_path, mappable_path_arguments, management: false| | |||
| shared_examples "mappable" do |mappable_factory_name, mappable_association_name, mappable_new_path, mappable_edit_path, mappable_show_path, mappable_path_arguments: {}, management: false| | |||
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [187/110] (https://rubystyle.guide#max-line-length)
We were using an optional parameter followed by keyword parameters, which caused a warning with Ruby 2.7: ``` app/components/shared/link_list_component.rb:20: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call ``` I've tried to make `current:` a named parameter as well and then change all method calls to `link_list`, but was still getting the same warning. Might have something to do with the fact that we're dealing with arrays with hashes inside them instead of passing the keyword arguments directly to the method.
We were passing a hash of options instead of converting them to keyword parameters, which resulted in warnings on Ruby 2.7: ``` app/components/admin/table_actions_component.html.erb:5: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call app/components/admin/table_actions_component.html.erb:9: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call app/components/admin/widget/cards/row_component.html.erb:20: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call app/components/admin/widget/cards/table_component.html.erb:14: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call ```
This method changed in Globalize version 6.0.1 [1], which we're using since commit 6072372. We were getting a deprecation warning in Ruby 2.7: ``` config/initializers/globalize.rb:8: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call ``` So we're using the `(...)` arguments syntax to make it compatible with Ruby 3.0, just like Globalize does when using Ruby 2.7. [1] See pull request 778 in the globalize/globalize GitHub repo.
We were getting a warning with Ruby 2.7 due to the change in the way keyword arguments are handled in Ruby 3.0. ``` ruby/gems/2.7.0/gems/rspec-support-3.11.0/lib/rspec/support/with_keywords_when_needed.rb:18: warning: Passing the keyword argument as the last hash parameter is deprecated ``` As hinted by the warning, this code crashes with Ruby 3.0: ``` ArgumentError: unknown keyword: :budget_id ``` I'm not sure why this is the case, though, since we were already explicitely passing a hash first before passing the keyword parameters. I guess there are some cases in this whole keyword VS hash parameters incompatibility that I haven't completely understood.
We were getting a warning with Ruby 2.7:
```
ruby/gems/2.7.0/gems/capybara-3.37.1/lib/capybara/session.rb:377:
warning: Using the last argument as keyword parameters is deprecated;
maybe ** should be added to the call
```
On Ruby 3.0, the test failed with `Unable to find fieldset
{:text=>"Draft phase"}` and we were also getting another warning:
```
Locator Hash:{:text=>"Draft phase"} for selector :fieldset must be an
instance of String or Symbol. This will raise an error in a future
version of Capybara
```
We were passing a hash when the method definition expected keyword arguments, and so we were getting warnings in Ruby 2.7: ``` db/dev_seeds/widgets.rb:11: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call db/dev_seeds/widgets.rb:23: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call db/dev_seeds/widgets.rb:35: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call db/dev_seeds/widgets.rb:47: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call db/dev_seeds/admin_notifications.rb:3: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call db/dev_seeds/admin_notifications.rb:11: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call db/dev_seeds/admin_notifications.rb:19: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call db/dev_seeds/admin_notifications.rb:27: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call ``` Converting the hash to keyword arguments solves the issue.
We've noticed the following warning while testing the upgrade to Ruby 3.0: warning: File.exists? is deprecated; use File.exist? instead We're adding a Rubocop rule so we don't call the deprecated method in the future.
While running the `dev_seed` twice, as we do in the tests, we were getting the following warnings: ``` db/dev_seeds/proposals.rb:1: warning: already initialized constant IMAGE_FILES db/dev_seeds/budgets.rb:1: warning: already initialized constant INVESTMENT_IMAGE_FILES ``` So we're extracting a method which allows us to use local variables while removing duplication. We had this warning with every version of Ruby, not just Ruby 2.7, but since we're getting rid of all the warnings, we're taking care of this one as well.
Now that we've got rid of all the warnings we had, we can enable them so we'll notice new warnings when we introduce them. This was the default option until Ruby 2.7.2 was released [1]. These warnings were turned off by default because pretty much every Ruby gem had dozens of warnings with Ruby 2.7 due to the changes in the way Ruby handles keyword arguments. [1] https://www.ruby-lang.org/en/news/2020/10/02/ruby-2-7-2-released/
javierm
force-pushed
the
ruby_3_warnings
branch
from
023b3f3
to
a4d6e23
Compare
taitus
approved these changes
Jan 31, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Objectives