-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[chore] make flickering disappear #17212
Conversation
- use test locale for localisation specs to avoid errors due to lazy i18n loading
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
after do | ||
I18n.config.enforce_available_locales = true | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I see the comment above, this will also leak, no? Would it be better to store the original value and reassign it here, or is that overkill?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, this is the original one. It is also the default. Not sure, if we should bother with variable assignment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's all good, you get to make a call. My idea was to be a bit extra defensive. If we (or Rails) ever decides to make a change to this default 🙃 which is very unlikely.
What are you trying to accomplish?
What approach did you choose and why?