Deprecate other code related to old factories loading

[kennyadsl]

Summary

Ref #5023

People should have started to use require 'spree/testing_support/factory_bot'now, so this workaround is no longer needed.

Also, no one is calling the methods that check for the version of factory_bot, so we can safely deprecate that code.

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

• I have written a thorough PR description.
• I have kept my commits small and atomic.
• I have used clear, explanatory commit messages.

The following are not always needed:

• 📖 I have updated the README to account for my changes.
• 📑 I have documented new code with YARD.
• 🛣️ I have opened a PR to update the guides.
• ✅ I have added automated tests to cover my changes.
• 📸 I have attached screenshots to demo visual changes.

[elia]

@kennyadsl would make sense to add a warning in the file instructing people to stop requiring it?

[kennyadsl]

@elia I tried but I think it's still loaded by Zeitwerk when someone require an inner module like 'Spree::TestingSupport::FactoryBot'. Do you know any way to avoid this?

[waiting-for-dev]

@elia I tried but I think it's still loaded by Zeitwerk when someone require an inner module like 'Spree::TestingSupport::FactoryBot'. Do you know any way to avoid this?

Are you sure? I tried adding a puts line in spree/testing_support.rb, and I required the spree/testing_support/factory_bot in a sandbox application with the factory_bot_rails gem added. I didn't see the puts output.

However, shouldn't we deprecate the method itself?

[kennyadsl]

Are you sure?

Nope, my attempt was removing the file completely and the CI was failing, but maybe thinking twice, it was for another reason, see here. Let me try again.

However, shouldn't we deprecate the method itself?

Yes, you are right, even if it's not used.

Considering that it's better to add deprecations to push people to not require this file at all, marking it and its content for removal, does this mean we can do this whole PR later after releasing v4.0? I think it does, no rush on this cleanup.

[waiting-for-dev]

Considering that it's better to add deprecations to push people to not require this file at all, marking it and its content for removal, does this mean we can do this whole PR later after releasing v4.0? I think it does, no rush on this cleanup.

We can also release another 3.4.x version with the warning and remove the file altogether on v4, but either way it's not something critical

[kennyadsl]

@waiting-for-dev This PR is now deprecating those methods/files instead of removing them. I think the idea to backport and release a patch is acceptable because we still didn't release v4.0.

[waiting-for-dev]

👍

[github-actions]

💔 All backports failed

Status	Branch	Result
❌	v3.4	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

backport --pr 5059

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details