search

igneus / calendarium-romanum

liturgical calendar library (Roman Catholic, post-Vatican II)
50 stars 21 forks source link

Kill I18n deprecation warning #47

Closed mkasberg closed 4 years ago

mkasberg commented 4 years ago

Before:

$ bundle exec calendariumrom query 2019-09-22
[deprecated] I18n.enforce_available_locales will default to true in the future. If you really want to skip validation of your locale you can set I18n.enforce_available_locales = false to avoid this message.
2019-09-22
season: Ordinary Time

Sunday : 25th Sunday in Ordinary Time

After:

$ bundle exec calendariumrom query 2019-09-22
2019-09-22
season: Ordinary Time

Sunday : 25th Sunday in Ordinary Time

I assume it would be preferable to set this value to true so an error is raised if a locale is not available.

https://stackoverflow.com/questions/20361428/rails-i18n-validation-deprecation-warning

mkasberg commented 4 years ago

I've used this library before, and wanted to make a few contributions as part of Hacktoberfest :beers:

No rush, take your time reviewing :slightly_smiling_face:

igneus commented 4 years ago

I agree with setting I18n.enforce_available_locales = true, but not in this place.

I strongly believe that a library should never modify the environment - like global settings of I18n - unless it is absolutely required for it's proper functioning. This should be left to the application code.

I18n.enforce_available_locales can (and probably should) be set there, where we "own" the environment - in the calendariumrom executable and in tests. But lib/calendarium-romanum/i18n_setup.rb is loaded whenever someone require 'calendarium-romanum', so it's definitely not the right place to do so.

mkasberg commented 4 years ago

That makes sense to me. I pushed up an amended commit.

igneus commented 4 years ago

Thank you very much, merging.