Skip to content

Conversation

@tommyh
Copy link
Owner

@tommyh tommyh commented Aug 30, 2022

Updating the documentation for setting self.ignored_columns on an ActiveRecord model to append to the existing value instead of overwriting the existing array.

Appending to the existing value, +=, is helpful for the scenario when 2 developers set ignored_columns in different parts of the file and the second one overwrites the first one.

Summary

Our team just hit a production issue where two developers set self.ignored_columns on the same ActiveRecord model, but they did it in different locations in the file. The code looked like this:

class Project < ActiveRecord::Base

  self.ignored_columns = [:category]

  [..... unrelated code here....]

  self.ignored_columns = [:name]

end

So the end result was:

[11] pry(main)> Project.ignored_columns
=> ["name"]

This PR updates the example documentation so that it is more likely developers would write this:

class Project < ActiveRecord::Base

  self.ignored_columns += [:category]

  [..... unrelated code here....]

  self.ignored_columns += [:name]

end

which would result in:

[11] pry(main)> Project.ignored_columns
=> ["category", "name"]

Other Information

There is a rubocop rails style code that recommends the "append" pattern for ignored_columns, but this is a style guide which a rails developer might not know about and/or read.

I will look into adding this to rubocop-rails as well, but that is an optional linter which doesn't have 100% adoption and a documentation fix seems simple enough that everyone can benefit from it.

Updating the documentation for setting `self.ignored_columns` on an ActiveRecord model to append to the existing value instead of overwriting the existing array.

Appending to the existing value, `+=`, is helpful for the scenario when 2 developers set `ignored_columns` in different parts of the file and the second one overwrites the first one.
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