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

Refactor fallbacks plugin #433

Merged
merged 3 commits into from
Aug 23, 2020
Merged

Refactor fallbacks plugin #433

merged 3 commits into from
Aug 23, 2020

Conversation

shioyama
Copy link
Owner

The fallbacks plugin is a total mess. For now, I'm just going to rip out the parts of it that pollute other namespaces, while leaving the existing usage intact. In another PR I'll update the usage once I've gone through the suite to fix hidden dependencies (a lot of specs actually fail if you remove fallbacks right now because they depend on it being their implicitly.)

# instance. By default this is a proc which takes fallbacks and returns an
# instance of +I18n::Locale::Fallbacks+.
# @param [Proc] fallbacks generator
attr_writer :fallbacks_generator
Copy link
Owner Author

Choose a reason for hiding this comment

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

I'm removing this, I doubt it's almost ever used and it's cluttering multiple unrelated parts of the codebase. Stuff like this will be configurable by overriding plugin methods on the Mobility::Attributes subclass. I've added a spec showing how to do this. Once I finish removing Configuration, this wil be easier to do directly from Mobility.configure.

@shioyama
Copy link
Owner Author

Some of this code is related to #161. I'm wondering if perhaps there should be a separate plugin called i18n_fallbacks which just does what #161 was asking for, so we can leave the original implementation clean 🤔

fallbacks_class.new(fallbacks)
end

class I18nFallbacks < ::I18n::Locale::Fallbacks
Copy link
Owner Author

Choose a reason for hiding this comment

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

Thinking maybe this should be its own plugin....

@shioyama shioyama merged commit 22f8b71 into master Aug 23, 2020
@github-pages github-pages bot temporarily deployed to github-pages August 23, 2020 03:47 Inactive
@shioyama shioyama deleted the cleanup_fallbacks branch August 28, 2020 05:38
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.

1 participant