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 rails 6.1 errors is now array warnings #8408

Merged

Conversation

jrafanie
Copy link
Member

Fixes this warning in several places in the test suite:

  In Rails 6.1, `errors` is an array of Error objects,
  therefore it should be accessed by a block with a single block
  parameter like this:

  person.errors.each do |error|
    attribute = error.attribute
    message = error.message
  end

  You are passing a block expecting two parameters,
  so the old hash behavior is simulated. As this is deprecated,
  this will result in an ArgumentError in Rails 7.0.

@Fryguy
Copy link
Member

Fryguy commented Aug 11, 2022

Is this code Rails 6.0 compatible, or will it have to be reverted if we need to revert 6.1?

@jrafanie
Copy link
Member Author

jrafanie commented Aug 11, 2022

Is this code Rails 6.0 compatible, or will it have to be reverted if we need to revert 6.1?

If I can ever get spec:javascript to fail locally and then pass in CI, I'll look to see if 6.0 will work with the array assumption. My hunch is this is a rails 7 change and we'd need to revert to go back to 6.0

@kbrock
Copy link
Member

kbrock commented Aug 11, 2022

Is this code Rails 6.0 compatible, or will it have to be reverted if we need to revert 6.1?

This is not rail 6.0 friendly.
It is a change that is available in 6.1 (optional)
and then mandatory in 7.0

Fixes this warning in several places in the test suite:
```
  In Rails 6.1, `errors` is an array of Error objects,
  therefore it should be accessed by a block with a single block
  parameter like this:

  person.errors.each do |error|
    attribute = error.attribute
    message = error.message
  end

  You are passing a block expecting two parameters,
  so the old hash behavior is simulated. As this is deprecated,
  this will result in an ArgumentError in Rails 7.0.
```
@jrafanie jrafanie force-pushed the fix_errors_object_now_array_in_6_1_warning branch from b80f0c1 to 3f3815e Compare August 12, 2022 13:58
@miq-bot
Copy link
Member

miq-bot commented Aug 12, 2022

Checked commit jrafanie@3f3815e with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
24 files checked, 1 offense detected

app/controllers/application_controller/ci_processing.rb

@kbrock
Copy link
Member

kbrock commented Aug 25, 2022

This is low risk, but I said Oparin/no
If we are worried about backport conflicts, then it may make sense to backport this

@jrafanie jrafanie deleted the fix_errors_object_now_array_in_6_1_warning branch August 25, 2022 20:39
@Fryguy
Copy link
Member

Fryguy commented Aug 30, 2022

Don't put oparin/no at the moment only because it will be backported anyway after we merge master into oparin after the stabilization period.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants