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

Remove unused attr_writers visitor and indexes #35117

Merged
merged 1 commit into from
Feb 1, 2019

Conversation

kamipo
Copy link
Member

@kamipo kamipo commented Jan 31, 2019

I deprecated two unused attr_writers visitor and indexes at 8056fe0
and f4bc364 conservatively, since those are accidentaly exposed in the
docs.

https://api.rubyonrails.org/v5.2/classes/ActiveRecord/ConnectionAdapters/AbstractAdapter.html
https://api.rubyonrails.org/v5.2/classes/ActiveRecord/ConnectionAdapters/TableDefinition.html

But I've found that view_renderer attr_writer is removed without
deprecation at #35093, that is also exposed in the doc.

https://api.rubyonrails.org/v5.2/classes/ActionView/Base.html

I'd like to also remove the deprecated attr_writers since I think that
removing visitor and indexes attr_writers is as safe as removing
view_renderer attr_writer.

I deprecated two unused attr_writers `visitor` and `indexes` at 8056fe0
and f4bc364 conservatively, since those are accidentaly exposed in the
docs.

https://api.rubyonrails.org/v5.2/classes/ActiveRecord/ConnectionAdapters/AbstractAdapter.html
https://api.rubyonrails.org/v5.2/classes/ActiveRecord/ConnectionAdapters/TableDefinition.html

But I've found that `view_renderer` attr_writer is removed without
deprecation at rails#35093, that is also exposed in the doc.

https://api.rubyonrails.org/v5.2/classes/ActionView/Base.html

I'd like to also remove the deprecated attr_writers since I think that
removing `visitor` and `indexes` attr_writers is as safe as removing
`view_renderer` attr_writer.
@kamipo kamipo merged commit 1d48a20 into rails:master Feb 1, 2019
@kamipo kamipo deleted the remove_unused_attr_writer branch February 1, 2019 00:07
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.

None yet

2 participants