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

PHP7ize Laravel 4.2 #13338

Closed
wants to merge 1 commit into
base: 4.2
from

Conversation

Projects
None yet
3 participants
@kampernet

kampernet commented Apr 27, 2016

No description provided.

Joel Klaverkamp

@kampernet kampernet closed this Apr 27, 2016

@kampernet

This comment has been minimized.

kampernet commented Apr 27, 2016

sorry, accidentally created a pull request to the 4.2 branch, am actually just trying to fork it and get it to run under PHP7 in a controlled way.

@GrahamCampbell

This comment has been minimized.

Member

GrahamCampbell commented Apr 27, 2016

Note that your changes here area breaking php 5.

@GrahamCampbell

This comment has been minimized.

Member

GrahamCampbell commented Apr 27, 2016

They are also breaking lots of packages by modifying method definitions.

@GrahamCampbell

This comment has been minimized.

Member

GrahamCampbell commented Apr 27, 2016

We fixed things in a better way on 5.1.

@kampernet

This comment has been minimized.

kampernet commented May 3, 2016

hey thanks Graham, do you mind pointing me to where you fixed things in a better way in 5.1? specifically how you made it runnable on both 5 and 7? I searched the code base and am pretty sure I found direct references to the Throwable interface, I may not be understanding you correctly.

@jonstavis

This comment has been minimized.

jonstavis commented May 14, 2016

@GrahamCampbell are there any plans/is it possible to apply a fix like this for 4.2? Would be awesome to deploy legacy apps with PHP7

@GrahamCampbell

This comment has been minimized.

Member

GrahamCampbell commented May 14, 2016

Only Laravel 5.1 is supported. We dropped support for 4.2 the same day 5.0 was released.

@kampernet

This comment has been minimized.

kampernet commented May 16, 2016

@jonstavis what I did in the end was extend the Illuminate\Exception\Handler class to do this:

        $e = $exception;
        if (!$exception instanceof Exception) {
            try {
                throw new Exception($exception);
            } catch(Exception $e) {
                // caught ya
            }
        }

        $displayer = $this->debug ? $this->debugDisplayer : $this->plainDisplayer;

        return $displayer->display($e);

so basically, if it's not an exception, throw it as an exception ( to cast it to an Exception )

then I just also extended the Illuminate\Foundation\Application class and the ExceptionServiceProvider class to use my new Handler

Not sure if this is the "correct" way to do it, but it means I can still use the 4.2 branch as it is, without forking it, and so far my tests are all passing...

@jonstavis

This comment has been minimized.

jonstavis commented May 17, 2016

Awesome! This is a great idea, I'll give it a shot. Thank you!

So the last bit of it is you just changed one line of bootstrap/start.php to

$app = new My\Custom\Application;

?

@kampernet

This comment has been minimized.

kampernet commented May 17, 2016

yes, exactly, and in that subclass, override the registerExceptionProvider method to use your ExceptionServiceProvider that overrides the registerHandler method to use your Handler that casts the Error / Throwable to an Exception.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment