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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combine FileSystemResolver classes #41969

Merged
merged 7 commits into from
Apr 14, 2021

Conversation

jhawthorn
Copy link
Member

This merges the previous three classes of OptimizedFileSystemResolver, FileSystemResolver, and PathResolver into a single FileSystemResolver which has the same behaviour as the previous OptimizedFileSystemResolver. After all, why wouldn't we want it always optimized 馃槈.

This is possible because we've removed the FallbackFileSystemResolver in #41846 and this PR removes support for template names including a "." (which was deprecated in Rails 6.1). This means that all supported behaviours are handled by the optimized version of the resolver.

Copy link
Contributor

@kaspth kaspth left a comment

Choose a reason for hiding this comment

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

Yes! 鉂わ笍

@kaspth
Copy link
Contributor

kaspth commented Apr 14, 2021

Looks like the failing assertion is this one:

assert_equal xml, assert_deprecated { render["respond_to/using_defaults.xml.builder"] }

Which fails because you've removed the deprecation, so we should be able to safely remove the assertion line too.

@jhawthorn jhawthorn force-pushed the combine_file_system_resolvers branch from 8e1ca9d to c70bd13 Compare April 14, 2021 18:27
@rails-bot rails-bot bot added the actionpack label Apr 14, 2021
@jhawthorn jhawthorn merged commit ec1735b into rails:main Apr 14, 2021
@jhawthorn jhawthorn deleted the combine_file_system_resolvers branch April 14, 2021 22:46
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