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

Eliminate extra template lookup in ActionView::Digestor #25826

Merged

Conversation

javan
Copy link
Contributor

@javan javan commented Jul 14, 2016

PathSet#exists? and PathSet#find both do the same thing (PathSet#find_all). The exists? avoids MissingTemplate errors when no template is found. This change avoids it by using find_all.first instead. No behavior change, just one less template lookup.

Noticed this while working on #25411.

/cc @dhh @matthewd

@dhh
Copy link
Member

dhh commented Jul 15, 2016

Sounds good to me. Any concerns @rafaelfranca ?

@rafaelfranca
Copy link
Member

:shipit: . We may want to change the API of PathSet to have a find and find! method so we don't raise an exception by default when trying to find the first template. But that can be a future refactoring.

@rafaelfranca rafaelfranca merged commit d41d7a4 into rails:master Jul 15, 2016
@rafaelfranca
Copy link
Member

Backported in 0ef05cd

rafaelfranca added a commit that referenced this pull request Jul 15, 2016
…e-lookups

Eliminate extra template lookup in ActionView::Digestor
@javan
Copy link
Contributor Author

javan commented Jul 15, 2016

❤️

We may want to change the API of PathSet to have a find and find!

That was my instinct too and I agree it should be a separate change.

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

3 participants