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

Disable rubocop cop "Rails/ActiveRecordAliases" #2517

Conversation

kristinalim
Copy link
Member

What? Why?

In #2512, rubocop complained that the code uses update_attributes! instead of update!. But the latter is not yet available in the Rails version that we are using.

See this for more information about the cop.

The cop documentation shows that these are the only methods it warns about:

ALIASES =
{
  update_attributes: :update,
  update_attributes!: :update!
}.freeze

Release notes

  • Disable Rubocop cop "Rails/ActiveRecordAliases".

Changelog Category: Fixed

How is this related to the Spree upgrade?

If the Spree upgrade includes a major upgrade of Rails, we should enable the cop again.

The methods :update! and :update (to replace :update_attributes! and
:update_attributes) were not added until Rails 4.

See this for more information:
https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Rails/ActiveRecordAliases
@kristinalim kristinalim force-pushed the cleanup-disable_rubocop_cop_rails_activerecordaliases branch from a748fa5 to d77a943 Compare August 6, 2018 10:45
Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Excellent. As this is not affecting code, I think I can merge this straight away.

@mkllnk mkllnk merged commit 32846af into openfoodfoundation:master Aug 6, 2018
@kristinalim kristinalim self-assigned this Aug 7, 2018
@kristinalim kristinalim deleted the cleanup-disable_rubocop_cop_rails_activerecordaliases branch August 30, 2018 01:45
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