Remove support for deprecated way of loading factories

solidusio / solidus_dev_support

A collection of tools for developing Solidus extensions.

MIT License

21 stars 27 forks source link

Remove support for deprecated way of loading factories #205

Closed kennyadsl closed 1 year ago

kennyadsl commented 1 year ago

Summary

Without this change, all the extensions will fail against Solidus v4.0 because the file spree/testing_support/factories that we always require no longer exists.

If any extension is still loading factories using require it should change to use load_for or will fail.

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.

mergify[bot] commented 1 year ago

It looks like this PR is missing a label to determine the type of change it introduces. The maintainer should add one of the following labels:

bug for bugfixes.
enhancement for new features and improvements.
documentation for documentation changes.
security for security patches.
removed for feature removals.
infrastructure for internal changes that should not go in the changelog.

Additionally, the maintainer may also want to add one of the following:

breaking for breaking changes.
deprecated for feature deprecations.

Once the correct labels have been set, simply remove the needs changelog label label from this PR so I can merge it.

kennyadsl commented 1 year ago

@waiting-for-dev can you take a second look now?

kennyadsl commented 1 year ago

@waiting-for-dev I changed the approach after discussing it with Elia. There's no need to support the old way of loading factories because if an extension does that, it will fail to test against master anyway. If that happens, the only thing we can do is update the extension to use load_for, which will make specs pass against all versions.

waiting-for-dev commented 1 year ago

@waiting-for-dev I changed the approach after discussing it with Elia. There's no need to support the old way of loading factories because if an extension does that, it will fail to test against master anyway. If that happens, the only thing we can do is update the extension to use load_for, which will make specs pass against all versions.

I was under the impression that we wanted to support old Solidus while an extension gets ready for master, but given that's something that will only affect development, I agree we should go with the simplest approach :+1: