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

[9.x] Add methods to Enumerable contract #46021

Merged
merged 1 commit into from
Feb 8, 2023
Merged

[9.x] Add methods to Enumerable contract #46021

merged 1 commit into from
Feb 8, 2023

Conversation

xurshudyan
Copy link
Contributor

Add missing methods to Enumerable contract PR: #45839

@taylorotwell taylorotwell merged commit 3b8057c into laravel:9.x Feb 8, 2023
@johanrosenson
Copy link
Contributor

is it not a breaking change to add methods to the interface?

@xurshudyan
Copy link
Contributor Author

is it not a breaking change to add methods to the interface?

You think it will break something?

@johanrosenson
Copy link
Contributor

Yes.

If anyone is implementing the interface their code will break because they don't have the new methods that was added.

New methods are usually only added to interfaces in major versions because of this.

You can look at the laravel 8 and 9 upgrade guides to see this.

8.x

8.x/upgrade#the-castable-interface
8.x/upgrade/#the-dispatcher-contract

9.x

9.x/upgrade#the-enumerable-contract
plus a bunch more in 9.x that can't be linked directly

driesvints added a commit that referenced this pull request Feb 9, 2023
driesvints added a commit that referenced this pull request Feb 9, 2023
@driesvints
Copy link
Member

I've reverted this PR as it indeed contains breaking changes to the method signature and adds methods to existing contracts. Please attempt to 10.x instead.

@xurshudyan
Copy link
Contributor Author

I've reverted this PR as it indeed contains breaking changes to the method signature and adds methods to existing contracts. Please attempt to 10.x instead.

Good, thanks

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.

4 participants