performance: remove unnecessary i18n locale reload

[codez]

Summary

I18n is reloaded whenever load_path is set, so an additional reload is not needed and only increases load time (because all locale files are reloaded twice 😞).

See https://github.com/ruby-i18n/i18n/blob/b4cd00d73234e369d713eb2607c729765bb3ec06/lib/i18n/config.rb#L135

Other Information

The steps described in the original issue that introduced the reload now work as expected, even without the reload.

[thdaraujo]

thanks for submitting this @codez!

I think your suggestion makes sense. But we had so many problems related to this in the past that I would like to have a test to make sure there's no regression.

[thdaraujo]

I tested main and I tested your change and they both seem fine. But I couldn't really reproduce the original issue anymore, so not sure. Here's what I've tried:

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem 'faker', :git => 'https://github.com/codez/faker.git', :branch => 'remove-unnecessary-i18n-reload', require: false
  # gem 'faker', :git => 'https://github.com/faker-ruby/faker.git', :branch => 'main', require: false

  gem "minitest"
end

require "minitest/autorun"
require "i18n"

class BugTest < Minitest::Test
  def test_faker
    refute I18n.backend.initialized?

    assert require 'faker'

    puts Faker::Name.name
    assert Faker::Name.name
  end

  def test_faker_forking
    refute I18n.backend.initialized?
    
    pid = fork do
      assert require 'faker'
      assert Faker::Name.name
    end
    
    _, status = Process.waitpid2(pid)
    assert status.success?, "Failed in forked process"
  end
end

Maybe someone has a better idea and is able to reproduce the original issue so we can know for sure.

[stefannibrasil]

hi @codez Thanks for your contribution! Like @thdaraujo mentioned, it's hard to keep changing this without being sure if it will still work, or why it wasn't working in the first place.

Could you please provide more reproduction steps and automated tests to help with the reproduction as well? Thanks!

[codez]

I added a test that loads faker in a subshell and asserts that the original issue now works.

Because the reloading is actually done in the i18n gem, this test also succeeds on the current main branch. The original issue is not reproducible with current versions of i18n.

[thdaraujo]

Thanks for adding tests! Left a suggestion on how to handle the exit status.

[thdaraujo]

CI is not passing

we'll need to fix the test to pass CI

[codez]

I adjusted the test, it should hopefully pass now.

[thdaraujo]

LGTM, thank you for adding tests!

[marcelobarreto]

LGTM