Documentation for "Working around good_migrations" is incomplete/inaccurate.

[armchairdj]

Per the README:

The gem only prevents auto-loading, so you can always can explicitly require the app code that you need in your migration.

But when attempting to require application code in a complex data migration, I was still blocked by the gem from running it. It was as if the require statements were not there.

Does this actually work? And if so, is there more precise documentation that could point users toward how to do it?

I ended up inlining > 100 lines of duplicated application code because I couldn't require a well-tested backfill service due to this issue.

Thanks for any help!

[searls]

I vaguely recall writing this, but I believe this is no longer true under zeitwerk and I'm not convinced it should be true. My inclination is to remove this from the README rather than figure out a way to work around Zeitwerk

[armchairdj]

FWIW, a teammate spelunked into your code and found this solution to allow vetted exceptions to Good Migrations rules for a single migration:

  def up
    GoodMigrations::PatchesAutoloader.instance.unpatch!

    # do some complex data migration using application code

    GoodMigrations::PatchesAutoloader.instance.patch!
  end

[searls]

Yeah, rather than rely on the particulars of which autoloader you're using and how they work, it would seem better to have an API like:

GoodMigrations.allow_me_to_require_stuff_because_my_app_is_tightly_coupled_to_this_migration_and_im_too_lazy_to_inline_it do
  require "…"
end

[jasonkarns]

Kinda like how strong_migrations defines safety_assured { }. though I've seen that wrapper just get pasted everywhere. :(

[searls]

Yeah, that's my motivation for giving it a scary name

[davidmyersdev]

I think the aphorism "perfect is the enemy of good" applies here. In a perfect world, I agree that this type of interface shouldn't be necessary, but things are rarely perfect in our line of work. I think these "escape hatches" are there for good reason; we can't assume to fully know and understand every scenario a consumer of these types of tools might encounter. Maybe it's laziness, or maybe it's that implementing tools like this, long after they are due, is harder than you might think. All that said, I'd be up for opening a PR to add this feature if you would accept it.

[searls]

Yes, if you send a PR with a similarly discouraging name (along the lines of minitest's i_suck_and_my_tests_are_order_dependent but without telling people they suck, maybe) + a test or two and I'd merge that!