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.

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 whitelisted_params config #65

Merged

Conversation

kennyadsl
Copy link
Contributor

This allows people to switch to the new config without breaking their applications or requiring a new release for libraries using this gem.

See solidusio/solidus#3865, which is still valid but merging this we allow people on older versions of Solidus to upgrade to newer versions of Rails.

This allows people to switch to the new config without
breaking their applications or requiring a new release
for libraries using this gem.
Even if compact, it has the same meaning.
@kennyadsl kennyadsl force-pushed the kennyadsl/deprecate-whitelisted_parameters branch from fe7de6c to 4fb8c63 Compare December 14, 2020 09:18
@panasyuk
Copy link

panasyuk commented Dec 14, 2020

@jumph4x Why would you need to remove this in minor version?

@jumph4x
Copy link
Owner

jumph4x commented Dec 14, 2020

@panasyuk honest mistake (I understand you meant patch version), simply lapsed on the fact that this would be a breaking change in Spree and solidus.

@jumph4x
Copy link
Owner

jumph4x commented Dec 14, 2020

@kennyadsl thank you for this PR. Normally I would issue just another patch version bump. Should we bump patch or minor for this one?

Because I made the mistake of cutting a patch version for the breaking change, it makes sense to make to patch this one as well, so that ~> 0.2.0 is reverted to a working state for everyone.

@jumph4x jumph4x merged commit b0801e2 into jumph4x:master Dec 14, 2020
@kennyadsl
Copy link
Contributor Author

Agree, patch level seems the best release to me. Thanks!

@panasyuk
Copy link

@jumph4x No, i meant exactly the change for "minor" version, which must not violate backward compatibility. Anyway, thank you for quick problem resolution. @kennyadsl, good job!

@jumph4x
Copy link
Owner

jumph4x commented Dec 15, 2020

@panasyuk I guess I do not fully understand you, because the removal happened in a patch version. Are we both talking about sem ver?

EDIT: I think I understand. You meant in a sense of patch being a subset of minor and having even more constraints. Like I said previously, simply an oversight on my part.

Thanks, all.

@jumph4x
Copy link
Owner

jumph4x commented Dec 15, 2020

https://rubygems.org/gems/canonical-rails/versions/0.2.11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants