[5.1] Give appropriate warning when changing columns if Doctrine DBAL is not available. #10002
Conversation
| @@ -279,6 +279,13 @@ protected function getDoctrineTableDiff(Blueprint $blueprint, SchemaManager $sch | |||
| */ | |||
| public function compileChange(Blueprint $blueprint, Fluent $command, Connection $connection) | |||
| { | |||
| if (! $connection->isDoctrineAvailable()) { | |||
| throw new \RuntimeException(sprintf( | |||
|
This is breaking so should probably go to 5.2 not 5.1. Don't do anything yet though. Taylor might not agree with me. Ping @taylorotwell. |
GrahamCampbell
changed the title
|
@GrahamCampbell Is this breaking? Is it because I added a method to a class? The other thing I could have done here would be to have done the class exists call in If it is breaking because it throws an exception, I'm not sure that is true. It would have / should have thrown an exception or fatal anyway; this just does it earlier than it would have with a more developer-friendly message. |
|
It's breaking because the behaviour has been changed. |
|
@GrahamCampbell ok. if throwing an exception instead of letting a fatal happen is enough to cause this to be a breaking change and if @taylorotwell says to target 5.2 instead i'm all in! :) |
|
What I'm saying is, sure, it's a breaking change, but, Taylor might be ok with it, given the context. :) |
…arning [5.1] Give appropriate warning when changing columns if Doctrine DBAL is not available.
Obvious in the docs is one thing. Obvious when writing code and you hadn't seen it in the docs is another. Somewhat related to #590, I thought it would have been super helpful if the exception I was shown in the console output had told me more about what was wrong than just fatalling because
Class 'Doctrine\DBAL\Driver\PDOMySql\Driver' not found.I could have put the exception in
getDoctrineConnection(and still could, probably) but I thought that addingisDoctrineAvailablewould let things further out do more meaningful messages when something requires Doctrine but it is not available. For example, I now seeChanging columns for table "lesson_templates" requires Doctrine DBAL; install "doctrine/dbal".which tells me exactly why Doctrine is required, not just that it needs to be installed.Happy to adjust this as needed. Happy to throw a more general exception in
getDoctrineConnectionas well, either as a part of this PR or in a follow-up PR.