j4mie / idiorm

A lightweight nearly-zero-configuration object-relational mapper and fluent query builder for PHP5.
http://j4mie.github.com/idiormandparis/
2.01k stars 369 forks source link

Refactor join() to implode() as of warning in PHP 7.4 #351

Closed henryruhs closed 4 years ago

henryruhs commented 5 years ago

Refactor join() to implode() as of deprecated warning in PHP 7.4... therefore some strict unit testing suites (like mine) that turn warnings into errors are going to fail.

While fixing PHPUnit to support 5.x up to 7.4 it turns out that we cannot support both. PHP 7.4 needs PHPUnit 8 at least - that release enforces return types for setUp and tearDown that will cause syntax error in 5.x...

So now this is an dead end accept we create a testing suite for 5.x and 7.4+

if (class_exists('\PHPUnit\Framework\TestCase')) {
    class PHPUnit_Framework_TestCase extends \PHPUnit\Framework\TestCase
    {
    }
}

Supported PHPUnit versions: https://phpunit.de/supported-versions.html

costasovo commented 4 years ago

Any progress on this issue? PHP 7.4 is coming :)

JanJakes commented 4 years ago

@redaxmedia This fixes PHP 7.4 deprecated join by introducing another PHP 7.4 deprecation:

implode(): Passing glue string after array is deprecated. Swap the parameters

You'd need to fix the order of implode() parameters so that the glue goes always first.

henryruhs commented 4 years ago

Sorry, I don't put more afford into this PR as long the maintainer is not present. Feels like I wasted my time here... 💁🏼‍♂️

treffynnon commented 4 years ago

I am here. I just haven't had time to look at this yet. I've just started a new job among a host of other things so...

henryruhs commented 4 years ago

@treffynnon Glad to have you here

Anyways, this PR needs a proper decision according to the dead end I described.

I suggest we open this project to PHP 7 refactoring based on community contributions and finally drop support for PHP 5. This would be Idiorm 2.x.x then...

Please :+1: or :-1:

treffynnon commented 4 years ago

So, my take on this is that where another project already answers this need it is not a particularly good use of time to reproduce it. In this case there is a derivative of Idiorm and Paris called Eloquent that features in the Laravel project, but can be used separately in place of Idiorm and Paris.

For a version 2 of Idiorm to get my support it would need to provide something that Laravel doesn't already. Either way users will have to change their code to the new API of either Idiorm version 2 or Eloquent so I struggle to see the benefit.

There is a good chance that I have missed an important use case, but I cannot think of it.

henryruhs commented 4 years ago

@treffynnon Like I said, this is a point were a decision has to be made. I am using Idiorm for a lightweight CMS - so refactoring the Symfony or Laravel query builder / ORM will cost months of refactoring and the project is losing one of it's key features. There are many stories like this to be told, so I am not asking for you doing all the work. I am asking to be open for a community driven version 2.

treffynnon commented 4 years ago

The decision was made on Thu, 29 Aug 2013 17:29:58 +0100 with a commit I made stating Idiorm was feature complete.


In the meantime I've tried to keep the tests working, but supporting every new PHP version where it would materially change the API was never going to be the goal. I also made numerous comments to this effect over the years and rejected a number of pull requests due to this.

Ultimately, not all software projects are supposed to live on. Sometimes there is a useful shelf life.

Whenever this discussion has been brought up before I've been very clear and recommended forking idiorm or using Eloquent. So far none have forked Idiorm to that extent.

henryruhs commented 4 years ago

I close this PR cause idiorm is just a dead peace of software - don't use it for future projects.

treffynnon commented 4 years ago

Thank you for the time you spent on this PR and I am glad that you have been able to use this software that Jamie and I wrote and maintained in our spare time for your project.

Idiorm and Paris are actively maintained, but their APIs will not change so, I disagree with your interpretation that this is a dead project. User's will continue to get use of out of Idiorm on older versions of PHP where systems cannot be upgraded or work is being completed in embedded systems. They will continue to be supported with bug fixes as the need arises.

My advice has been to use Eloquent instead for quite sometime so I agree with you that new projects should not use Idiorm or Paris. Projects that seek to upgrade to incompatible PHP versions should also move to a new project for their data access needs. Given the similarities Eloquent would be the most obvious choice and less work to complete the migration IMHO.

CatalinFrancu commented 4 years ago

Apologies for dwelling on a subject that has already been addressed several times. I believe that Eloquent lacks one crucial advantage that Idiorm/Paris have: ease of use. The Idiorm/Paris instructions are as easy as they come: drag two files anywhere in my code base and require them. Done.

Standalone use of Eloquent is not documented anywhere officially (or my Google-fu is not strong enough). There are instructions but they are significantly more complex by comparison.

So I thank you for this project (it saved me thousands of CPU hours in production!) and I encourage you to keep it up to date, at least the low-hanging fruit.