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

Laravel 5.x compatibility fixes #9

Closed
wants to merge 1 commit into from

Conversation

mxdfulder
Copy link

Most problems were in HasAttributes trait. I've collected missing methods and properties in HasAttributesCompatibility trait and added some missing Interfaces and Exception. All existing tests are passing now for both Laravel 5.x and 8.x version, but it could be some problems with dates casting for old Laravel versions.

@Radiergummi
Copy link
Member

Hey @mxdfulder, please don't think I've forgotten! I'm just having mixed feelings about this situation. Copy-pasting Eloquent code to support legacy versions adds quite the maintenance burden on us.
I'd favor adding the compatibility traits to the current revision, release a final version 2 to packagist, then release a new major version 3 with a minimum Laravel 7 requirement. That would mean you'd be locked in on elasticsearch@2.x if you choose to stay on Laravel 5.7, but have the option of upgrading in the future.
Would that work for you?

@Radiergummi Radiergummi self-assigned this Mar 16, 2021
@mxdfulder
Copy link
Author

Hey @mxdfulder, please don't think I've forgotten! I'm just having mixed feelings about this situation. Copy-pasting Eloquent code to support legacy versions adds quite the maintenance burden on us.
I'd favor adding the compatibility traits to the current revision, release a final version 2 to packagist, then release a new major version 3 with a minimum Laravel 7 requirement. That would mean you'd be locked in on elasticsearch@2.x if you choose to stay on Laravel 5.7, but have the option of upgrading in the future.
Would that work for you?

Yes, I think it's reasonable solution

@Radiergummi
Copy link
Member

Radiergummi commented Apr 1, 2021

Integrated in v2.3.0. Thanks anyway :)!

@Radiergummi Radiergummi closed this Apr 1, 2021
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.

2 participants