-
Notifications
You must be signed in to change notification settings - Fork 28
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
Should adding a method always trigger MAJOR change? #86
Comments
Imagine you're using class X. You inherit from X and add a method Now X gets updated and it suddenly has a method Exceptions are |
I don't think that the first release will support logic that consider the hierarchy of the scanned classes. For the moment, it is only analyzing the files that have changes which reduce the amount of time spent analyzing the code. To be able to analyze a change such as the one discussed here, it would have to load the ancestors in order to determine the available methods, which is not something we do at the moment (everything is analyzed through PHP Parser and no classes are actually loaded per se). I'm adding this to the next major release as it is something I also think would be worth having, but will require a different approach than the currently implemented solution. |
@nochso I understand what you're saying, but I don't think that should trigger major flag until X is updated in a way you talked about, if even then. Major flag should be raised only when backwards compatibility is broken. Scenario you are talking about, I don't think is about that, but rather about code quality. While that's definitely an important issue, probably it's not something we can expect semver checker library to take care of. @tomzx sounds good, thanx |
Hi all, I'm another believer that adding methods should be MINOR (although I admit that technically they could be a breaking change if someone has extended the class and added a new method with the same name, but that's an edge case and from what I've seen in the community (eg symfony's BC promise) it's not considered as breaking BC. I've opened #93 to change this, feel free to chime in. |
I've learned here that php-semver-checker can already override any individual rule to be any level you want, so defaulting to the strictest possible interpretation of the semantic versioning rules is probably a good starting point since anyone can relax any particular rule they want to, depending on their use case. I recommend closing this issue. |
Per V015 it makes sense when a method is being overridden, but if parent class does not contain it or there is no parent class, this should probably be a MINOR change, since there are no backwards compatibility issues. Right?
The text was updated successfully, but these errors were encountered: