Crud: Cviebrock/eloquent-sluggable drops support for php < 7.0

Created on 14 Sep 2017  路  3Comments  路  Source: Laravel-Backpack/CRUD

Our signature in https://github.com/Laravel-Backpack/CRUD/blob/master/src/ModelTraits/SpatieTranslatable/SluggableObserver.php

protected function generateSlug(Model $model, $event)

His signature in https://github.com/cviebrock/eloquent-sluggable/blob/master/src/SluggableObserver.php

 protected function generateSlug(Model $model, string $event)

Typehinting a string.

Dropping support for php 5.6 is a big decission. But until we do we are stuck with version 4.2.5 of his package.

To install laravel 5.5 you need php >= 7.0 anyway.

The fix is simple, but we should drop the suport for 5.6 in travis.

What do you think ?

breaking change

Most helpful comment

Hi guys,

Yup, totally agree on this one. Dropping PHP <7 support is a big decision, but I think the time has come. I myself am in the same boat - I have laravel/backpack apps that would be a pain to upgrade, but I think we shouldn't sacrifice security and maintainability for the sake of lazyness.

The plan is for the next backpack version (base 1.0.0 and crud 3.3.x) to only support Laravel 5.5, which requires PHP 7+. You can already see that happening on the Backpack\Base upgrade brach.

I know not everyone will agree with this decision, but this should:

  • eliminate conditional behaviour depending on PHP version;
  • allow us to rewrite a lot of the codebase using the cool new features in PHP 7;
  • make it sooo much easier to maintain the packages;
  • help backpack developers use the latest versions of the dependencies; @gotrecillo you mentioned sluggable, but there are plenty others - namely everything from spatie that _already_ require PHP 7+;
  • still provide a working version for developers who can't upgrade; just no new features;

I'll close the issue as the decision, in my mind, is already taken. But please let me know if you agrees/disagree - I make mistakes all the time and I'm happy to take back decisions if new information surfaces.

Cheers!

All 3 comments

Dropping support for PHP 5.6 is almost certainly a breaking change and something that @tabacitu at least should review.

On a _personal_ note (taking my hat off here as a maintainer), I do wish customers and others would upgrade their PHP and Laravel but sometimes there are very compelling reasons for them not to do so.

Laravel is always pretty progressive with it's requirements, adopting new technologies as fast as they come out. If a Laravel package is going to keep up, I'd say you want to bump to PHP7, or this type of thing will keep happening.

Hi guys,

Yup, totally agree on this one. Dropping PHP <7 support is a big decision, but I think the time has come. I myself am in the same boat - I have laravel/backpack apps that would be a pain to upgrade, but I think we shouldn't sacrifice security and maintainability for the sake of lazyness.

The plan is for the next backpack version (base 1.0.0 and crud 3.3.x) to only support Laravel 5.5, which requires PHP 7+. You can already see that happening on the Backpack\Base upgrade brach.

I know not everyone will agree with this decision, but this should:

  • eliminate conditional behaviour depending on PHP version;
  • allow us to rewrite a lot of the codebase using the cool new features in PHP 7;
  • make it sooo much easier to maintain the packages;
  • help backpack developers use the latest versions of the dependencies; @gotrecillo you mentioned sluggable, but there are plenty others - namely everything from spatie that _already_ require PHP 7+;
  • still provide a working version for developers who can't upgrade; just no new features;

I'll close the issue as the decision, in my mind, is already taken. But please let me know if you agrees/disagree - I make mistakes all the time and I'm happy to take back decisions if new information surfaces.

Cheers!

Was this page helpful?
0 / 5 - 0 ratings