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

Rename ActionView::Base#run #35627

Merged
merged 1 commit into from Mar 15, 2019

Conversation

sebjacobs
Copy link
Contributor

@sebjacobs sebjacobs commented Mar 15, 2019

Summary

There was a recent change by @tenderlove to Action view which introduced ActionView::Base#run [1].

We ran into an issue with our application because one of the core concepts in our domain model is a Run which is exposed in most of our views as a helper method also called run, which now conflicts with this new method.

In order to save us having to rewrite a substantial part of our app I was wondering if we might be able to rename this method instead to something like #call or perhaps #exec?

[1] c740ebdaf5

@rails-bot rails-bot bot added the actionview label Mar 15, 2019
@sebjacobs sebjacobs changed the title Rename ActionView::Base#run to #call Rename ActionView::Base#run Mar 15, 2019
Copy link
Member

@tenderlove tenderlove left a comment

Choose a reason for hiding this comment

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

Can you change it to _run? It's definitely not a public method, and I think the _ prefix would help to avoid helper method collisions.

@sebjacobs
Copy link
Contributor Author

Can you change it to _run? It's definitely not a public method, and I think the _ prefix would help to avoid helper method collisions.

That makes a lot more sense! cheers Aaron!

There was a recent change by @tenderlove to Action view which introduced
`ActionView::Base#run` [1].

We ran into an issue with our application because one of the core
concepts in our domain model is a `Run` which is exposed in most of our
views as a helper method, which now conflicts with this new method.

Although this is a public method it is not really meant to be part of
the public API.

In order to discourage public use of this method and to reduce the
chances of this method conflicting with helper methods we can prefix
this method with an underscore, renaming this method to `_run`.

[1] rails@c740ebdaf5
@sebjacobs
Copy link
Contributor Author

sebjacobs commented Mar 15, 2019

@tenderlove I have fixed up this branch based on your feedback. Thanks for responding so quickly!

@tenderlove tenderlove merged commit 905018f into rails:master Mar 15, 2019
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