Split and move around exceptions, make MigrationException an interface #636
Conversation
| 'Migrations configuration file already loaded' | ||
| ), | ||
| 8 | ||
| ); |
There was a problem hiding this comment.
You only really need to linebreak either self or sprintf, not both
| sprintf( | ||
| 'Migrations configuration file already loaded' | ||
| ), | ||
| 8 |
There was a problem hiding this comment.
What are those codes about? Do you think people might rely on them? Should they be documented somewhere?
There was a problem hiding this comment.
Ah I see, they were there before, let's keep them then, I suppose.
There was a problem hiding this comment.
You will have to ask someone else for that. :P 5d6fbf8
Just kept those for compatibility.
There was a problem hiding this comment.
OF course, with this new exception layout it's obsolete.
| sprintf( | ||
| 'Migration class "%s" contains a prepared statement. | ||
| Unfortunately there is no cross platform way of outputing it as an sql string. | ||
| Do you want to write a PR for it ?', |
| public static function new() : self | ||
| { | ||
| return new self( | ||
| sprintf( |
There was a problem hiding this comment.
sprintf() here is not really needed
|
I further split Doctrine\Migrations\Configuration\Exception\NotValid to specific ones and removed it. |
|
Ready to merge? |
|
Still wanted @alcaeus to have a look. 😊 But if you need it in for some other work, could be discussed after merge. |
|
Hmm, I alegedly decreased coverage by 1% just by moving around the methods and it's now blocking merge. 🤔 |
This follows concept introduced in ORM (doctrine/orm#6743 + doctrine/orm#7210) and naming follows pattern accepted in Doctrine CS.
No polyfill is currently provided in this PR, it should be introduced in 1.x afterwards.
closes #621
cc @greg0ire