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

Functions Added To Interfaces Must Be A Major Version Bump #10

Closed
GrahamCampbell opened this issue Jan 11, 2015 · 11 comments
Closed

Functions Added To Interfaces Must Be A Major Version Bump #10

GrahamCampbell opened this issue Jan 11, 2015 · 11 comments
Milestone

Comments

@GrahamCampbell
Copy link
Contributor

No description provided.

@tomzx
Copy link
Owner

tomzx commented Jan 11, 2015

Are methods added to an interface necessarily a major version bump?

From what I understand of semver, as long as you are not breaking anything (making incompatible API changes), a minor bump is good enough.

@GrahamCampbell
Copy link
Contributor Author

If someone is implementing an interface and you add a method, you cause a php fatal error.

@drgomesp
Copy link

Well, if you add a new method to an interface you will most likely get that error before committing the source code, which means you either fix the issue by implementing the method or moving it to a different interface. I think its still not a major version bump.

@GrahamCampbell
Copy link
Contributor Author

@drgomesp You're missing the point. If I have an interface that other people are using, I must not add methods to it. It will break their code. We MUST bump the major version.

@GrahamCampbell
Copy link
Contributor Author

The fact that this requires a major version bump isn't up for discussion - it does. I'm reporting this as a missing check in this software.

@tomzx tomzx modified the milestone: Candidate for next Minor Jan 11, 2015
tomzx added a commit that referenced this issue Jan 14, 2015
Renamed ClassMethodParameterMismatch to ClassMethodParameterChanged.
Bump composer branch-alias to 0.2-dev.
Added various fixtures to tests/fixtures.
Initial code for Interface and Trait.
[#10] Adding methods to an interface should generate a MAJOR
tomzx added a commit that referenced this issue Jan 14, 2015
Renamed ClassMethodParameterMismatch to ClassMethodParameterChanged.
Bump composer branch-alias to 0.2-dev.
Added various fixtures to tests/fixtures.
Initial code for Interface and Trait.
[#10] Adding methods to an interface should generate a MAJOR
tomzx added a commit that referenced this issue Jan 14, 2015
[#17] Verification codes
Point toward a custom branch of php-parser (hopefully temporarily).
@tomzx tomzx closed this as completed Jan 14, 2015
@GrahamCampbell
Copy link
Contributor Author

Also, changing the method signature of any method in an interface must be a major version bump too.

@GrahamCampbell
Copy link
Contributor Author

We can't modify typehints or add params etc.

@tomzx
Copy link
Owner

tomzx commented Jan 14, 2015

@GrahamCampbell If you take a look at the Ruleset, this is already covered by V035 and V036.

@GrahamCampbell
Copy link
Contributor Author

Ok, cool. I'll take a look at that in more detail at the weekend. :)

@rquadling
Copy link

With regard to a non stable API (i.e. a V0.x.x), is a minor version bump the way to go when adding a new method to an interface?

@tomzx
Copy link
Owner

tomzx commented Jul 6, 2016

@rquadling From my understanding of the spec, while you're developing a non-stable API, what qualifies as a major bump should be a minor version bump until you want to turn the API into a stable version 1.0.0. However, since php-semver-checker is unaware of the current semantic version, it cannot suggest you a minor version bump.

However, if you use php-semver-checker-git, the suggest command will return a minor bump: https://github.com/tomzx/php-semver-checker-git/blob/master/src/PHPSemVerCheckerGit/Console/Command/SuggestCommand.php#L131:L141.

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

No branches or pull requests

4 participants