-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecate Paperclip Adapter #5100
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to change CI for the paperclip adapter so the generator is run, right? 🤔
|
||
class_option :app_name, | ||
type: :string, | ||
default: 'MyStore', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can automate that with Rails.application.class.module_parent.name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was on my metal TODO list, and of course, I forgot about it. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
metal TODO list
🤘
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol 🤘
At the moment, the paperclip CI jobs should use the old adapter, which I don't think it's working because tests are failing and I don't understand why. Your proposed approach seems interesting, this way we'll stop using the deprecated adapter and just rely on the generated files. I'll give it a try. |
a27f81a
to
995172c
Compare
3f18496
to
87bc6c3
Compare
This generator allows incorporating the paperclip adapter into the host application, in order to remove support for it from core. This generator does two things: 1. Copy the Image and Taxon Paperclip adapters into the host application; 2. Set the right configurations in the spree.rb (path configurable) initializer in order to use the newly created files.
We are also providing a message with instructions for who wants to keep using Paperclip. We are silencing deprecation warnings when setting the deprecated configuration in the CI. This will allow us to keep testing if our code still works with the deprecated adapter.
When `--active_storage=false` is used in for the main installer we can use the new generator to copy the adapter in the host application.
Only when we are testing against Paperclip in the build matrix.
87bc6c3
to
7442e93
Compare
Summary
Closes #5033
Add
solidus:paperclip_adapter:install
generator, which allows incorporating the paperclip adapter into the host application, in order to remove support for it from the core.This generator does two things:
This PR also deprecates the Paperclip adapters, so that we can remove it in the next major version.
TODOs
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: