search

ruby-i18n / i18n

Internationalization (i18n) library for Ruby
MIT License
986 stars 411 forks source link

Only deep_symbolize_keys when needed #588

Closed paarthmadan closed 2 years ago

paarthmadan commented 2 years ago

Following the tentative changes proposed in #587 and #583, we should avoid performing deep_symbolize_keys on the translations when it's unnecessary.

This PR currently includes changes from #587 to showcase usage. I'll update it with changes from #583 too, if and when that merges.

This PR returns an additional attribute from the load_* methods to provide a flag of whether the keys have been symbolized at parse time. If so, we skip deep_symbolize_keys. This is a performance optimization that helps avoid traversing the object graph when we don't need to.

I suppose we can also inject a magic key in the locale hash indicating whether keys are symbolized already, but I felt this approach was less intrusive.

radar commented 2 years ago

I looked at that failing spec but it seems to be a flaky one, nothing caused by your changes here.

changes LGTM, will probably merge tomorrow morning or Tuesday, depending on my schedules

paarthmadan commented 2 years ago

@radar Could this PR, along with #583 and #587 be reconsidered? I've addressed all of your feedback. Thanks!

radar commented 2 years ago

Apologies for the delay here, it’s been a busy time at work. I will endeavour to look at these issues later this morning. On 22 Jan 2022, 05:08 +1100, Paarth Madan @.***>, wrote:

@radar Could this PR, along with #583 and #587 be reconsidered? I've addressed all of your feedback.Thanks! — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.Message ID: @.***>