Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Backport #3913 to V2.11 #4174

Merged
merged 1 commit into from
Mar 3, 2022
Merged

Backport #3913 to V2.11 #4174

merged 1 commit into from
Mar 3, 2022

Conversation

spaghetticode
Copy link
Member

Description

This PR backports #3913 to Solidus 2.11.

In order to avoid tedious warnings on Ruby 2.7 and greater, we need to switch to the maintained fork of paperclip on 2.11 as well. The deprecation is now fixed in kt-paperclip 6.4.x.

Checklist:

@spaghetticode spaghetticode self-assigned this Sep 21, 2021
@kennyadsl
Copy link
Member

I'm not totally convinced about changing this dependency on a patch-level release. Are we sure using kt-paperclip at the version allowed is preserving full backward compatibility with paperclip?

@spaghetticode
Copy link
Member Author

@kennyadsl if it helps, I can test it briefly on a real project (specs there were already green, but I'm unsure how much Paperclip is actually tested VS mocked there) unless your issue is with the major version requirements jump (from >= 4.3 to ~> 6.3).

In regards to breaking changes with the last version of Paperclip at 6.1, the new project seems to be following semantic versioning, their changelog includes only fixes and small functionality additions, so it looks safe to me from that point of view.

@spaghetticode spaghetticode mentioned this pull request Oct 25, 2021
Closed
@spaghetticode
Copy link
Member Author

spaghetticode commented Oct 25, 2021 •
edited
Loading

We discussed this internally, and we agreed that the best option would be to have a kt-paperclip release with the fix also for paperclip 4.x so we don't have to break semver for Solidus 2.11 by incorporating breaking changes from external dependecies. I've just opened an issue in order to verify if this is feasible.

tvdeyen
tvdeyen approved these changes Dec 16, 2021
View reviewed changes
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a good thing, even in a patch level

@filippoliverani @spaghetticode
Switch to maintained Paperclip fork
f4739c9 
Recently, kt-paperclip 4.4.0 was released, so we can safely switch
to the maintained kt-paperclip fork while keeping the gem on the
same major version.
@spaghetticode spaghetticode force-pushed the spaghetticode/backport-3913 branch from 8dd1758 to f4739c9 Compare February 22, 2022 15:14
@spaghetticode
Copy link
Member Author

I'm happy to announce that kt-paperclip maintainers have released 4.x and 5.x versions, so we can now avoid bumping to a major version with this PR 🎉

@spaghetticode spaghetticode merged commit c5e9b55 into solidusio:v2.11 Mar 3, 2022
@spaghetticode spaghetticode deleted the spaghetticode/backport-3913 branch March 3, 2022 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jarednorman jarednorman jarednorman approved these changes

@tvdeyen tvdeyen tvdeyen approved these changes

Assignees

@spaghetticode spaghetticode

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@spaghetticode @kennyadsl @tvdeyen @jarednorman @filippoliverani