search

concretecms / concrete5-legacy

Legacy repository for concrete5
http://www.concrete5.org
559 stars 323 forks source link

Php7 fixes #1927

Closed Remo closed 7 years ago

Remo commented 9 years ago

replaces #1917

use at your own risk! Those changes aren't made the proper way, this is merely a preview and not recommended for production! See comments by Korvin over here https://github.com/concrete5/concrete5-legacy/pull/1917#issuecomment-155906027

Remo commented 9 years ago

I had a quick look at what would be required to keep warnings on. Right at the first page I see 4 warnings:

Warning: Declaration of Concrete5_Model_Page::add(CollectionType $ct, $data) should be compatible with Concrete5_Model_Collection::add($data) 
Warning: Declaration of Concrete5_Model_Page::duplicate($nc, $preserveUserID = false) should be compatible with Concrete5_Model_Collection::duplicate()
Warning: Declaration of Concrete5_Model_CollectionAttributeKey::getList() should be compatible with Concrete5_Model_AttributeKey::getList($akCategoryHandle, $filters = Array)
Warning: Declaration of Concrete5_Model_CollectionAttributeKey::add($at, $args, $pkg = false) should be compatible with Concrete5_Model_AttributeKey::add($akCategoryHandle, $type, $args, $pkg = false)

We could use dynamic arguments and then check which one have a value and do the magic. Super ugly.. We could remove some of the methods in the parent class or rename them, but that will most likely break a few things.

Here's an example using renamed methods to avoid warnings. It's incomplete and might break existing code! https://github.com/ortic/concrete5-legacy/commit/9432ed3fb8244ab23ffcd7963847980ac25389cb

If anyone has a better suggestion, please let me know!

Remo commented 9 years ago

This page /dashboard/pages/attributes/edit/1/ seems to have a problem with the new include method:

`Warning: include(C:\Work\concrete5-legacy\web\concrete/attribute/type_form_header.php): failed to open stream: No such file or directory in C:\Work\concrete5-legacy\web\concrete\core\libraries\attribute\view.php on line 145`
mlocati commented 9 years ago

Well, I guess we'd need to do for 5.6 all the LSP fixes we've done for 5.7...

pekkanikolaus commented 7 years ago

Is there a chance this is going to see any progress in the foreseeable future? I'd love to put in the work myself, but can't at the moment....

mlocati commented 7 years ago

@pekkanikolaus I don't think anyone will spend so much time on the legacy version of concrete5. BTW @Remo expressed some interest in this

Remo commented 7 years ago

@pekkanikolaus I tried to find some people who are willing to write code or spend money to make this happen. So far I only found one customer who would pay a small amount of money, but by far not enough. Maybe things will start to change once php 5.6 is getting closer to EOL, but till then I don't think it's going to happen.

pekkanikolaus commented 7 years ago

@mlocati @Remo yeah, I totally understand. I would like to do a major site relaunch and use 5.6.3.4 for it (here is some background discussion on why). Our provider still supports PHP 5.6 and it'll be a while until they don't, so there is no urgent need, but I want this relaunch to be as future-proof as possible.

I also thought of a bounty to get the fixes done, but I would probably also not be able to get nearly enough money greenlighted to make it happen. @Remo - do you have a rough idea of how much it would take? If you do, feel free to drop me an E-Mail. My address is in my profile.

If we can't get it done right now, then maybe I'll trust there will be enough interest once PHP 5.6 reaches end of life and panic opens everyone's pursestrings, and go ahead with the relaunch :)

danielgasser commented 7 years ago

@Remo I'd be interested in helping make it work, cause I think it would still be less work than migrating sites (big ones) from c5 5.6.x to v8.x. Do you still have any interests in doing this?

Remo commented 7 years ago

@danielgasser yes, got some information I can't share here, I'll call you in ~10min okay?

JeRoNZ commented 7 years ago

I'd be interested in having 5.6 run on PHP7. I have quite a few sites that simply won't be upgraded, so I would be up for helping make it work.

Remo commented 7 years ago

@JeRoNZ if you create pull requests I'd be happy to look at them! The goal would be to show warnings again and fix the code where necessary. LSP changes should be made in accordance with 5.7 wherever possible, I'm pretty sure we'll have to rename some methods.

pekkanikolaus commented 7 years ago

FYI I started a forum discussion about this https://www.concrete5.org/community/forums/chat/anyone-else-prefer-5.6s-ui-over-5.7s-and-5.8s-does-5.6-have-a-fu

Remo commented 7 years ago

Closed in favour of https://github.com/concrete5/concrete5-legacy/pull/1955