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

Breaking changes in newer version/s #93

Open
adriancb opened this issue Aug 17, 2021 · 2 comments
Open

Breaking changes in newer version/s #93

adriancb opened this issue Aug 17, 2021 · 2 comments

Comments

@adriancb
Copy link

As a follow-up to #92 there are several recent PATCH releases that may have changed the 'public' interface and therefore do not adhere to the correct SEMVER - https://semver.org/.

An example is extending the controller and adding to the resource session https://github.com/oivoodoo/devise_masquerade/blob/v1.3.3/app/controllers/devise/masquerades_controller.rb#L17.

@oivoodoo
Copy link
Owner

oivoodoo commented Nov 5, 2022

Hi @adriancb

Thank you for reporting the issue but could you clarify the required changes that I need to apply?

@adriancb
Copy link
Author

adriancb commented Nov 6, 2022

Hey @oivoodoo,

I believe the thought behind this was the renaming of find_resource to find_masqueradable_resource. As protected methods, it's reasonable to expect that inheriting classes can overload them (i.e. provide their own implementation).

IMO, the changes from 1.3.3 to 1.3.4 should've triggered a major release as they're not backwards compatible.

A change from protected > private (even though this is Ruby) at least marks them as 'private', which shifts the responsibility to those overloading those methods to understand the future impact.

The reason I'm bringing this up is that we upgrade our libs daily, and we spend much of our focus on reviewing major/minor version changes with less focus on patch changes, as we take them for granted. In this instance, we were unaffected, but that could've been different.

Let me know if that doesn't make sense or what your thoughts are.

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

No branches or pull requests

2 participants