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

Fix deprecation warnings #259

Merged
merged 2 commits into from
Aug 22, 2022

Conversation

yujideveloper
Copy link
Contributor

I fixed two deprecation warnings below.

  • DEPRECATION WARNING: Rails 7.0 has deprecated Enumerable.sum in favor of Ruby's native implementation available since 2.4. Sum of non-numeric elements requires an initial argument.
  • DEPRECATION WARNING: Time#to_s(:number) is deprecated. Please use Time#to_fs(:number) instead.

```
DEPRECATION WARNING: Rails 7.0 has deprecated Enumerable.sum in favor of Ruby's native implementation available since 2.4. Sum of non-numeric elements requires an initial argument.
```
```
DEPRECATION WARNING: Time#to_s(:number) is deprecated. Please use Time#to_fs(:number) instead.
```
@@ -13,7 +13,7 @@ def data_from_multiple_files
loaded_files = full_paths.collect { |path| load_path(path) }

if loaded_files.all?{ |file_data| file_data.is_a?(Array) }
loaded_files.sum
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this something we can fix in the definition of sum?

Or is this just a place where the method signature has been messed up in upstream rails?

Copy link
Collaborator

Choose a reason for hiding this comment

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

your comments explain that enough.

ignore.

(but still want to say darn)

@kbrock
Copy link
Collaborator

kbrock commented Aug 17, 2022

@yujideveloper do you know what the business process is around the review/merge?

Am I allowed to review and merge your changes?
Or do we need 2 different people on that?
With this project, I worry that requiring 2 different reviewers would mean nothing gets merged

@kbrock kbrock merged commit e0edeb4 into active-hash:master Aug 22, 2022
@yujideveloper yujideveloper deleted the fix/deprecation-warnings branch August 23, 2022 13:43
@yujideveloper
Copy link
Contributor Author

@kbrock

Sorry, late reply.

do you know what the business process is around the review/merge?

I don't know it in this project.

Am I allowed to review and merge your changes?
Or do we need 2 different people on that?
With this project, I worry that requiring 2 different reviewers would mean nothing gets merged

I think it's better to have two different reviewers. But as you say, I think nothing will be merged in this project now.

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