Skip to content
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

Relax return types of getExceptionConverter() implementations #6221

Merged

Conversation

derrabus
Copy link
Member

Q A
Type improvement
Fixed issues N/A

Summary

This takes back a change I made with #4926. If I want to implement a new let's say MySQL driver that uses a custom exception converter, I'm forced to extend the MySQL exception converter which feels too restrictive.

Background: I maintain a custom ODBC driver for DBAL and since ODBC has its own error codes, the MySQL exception converter is useless for me in that context.

@derrabus derrabus added this to the 4.0.0-RC2 milestone Nov 14, 2023
@derrabus derrabus requested review from morozov and greg0ire November 14, 2023 09:55
@@ -52,7 +53,7 @@ public function getDatabasePlatform(ServerVersionProvider $versionProvider): Abs
return new MySQLPlatform();
}

public function getExceptionConverter(): ExceptionConverter
public function getExceptionConverter(): ExceptionConverterInterface
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one was not narrowed in #4926, it's just as narrow on 3.x… I think it's fine not to document that change as breaking since ExceptionConverter is internal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the type is wider on 3.x.

public function getExceptionConverter(): ExceptionConverter
{
return new MySQL\ExceptionConverter();
}

I'm restoring the 3.x return type declaration.

@derrabus derrabus merged commit 8861620 into doctrine:4.0.x Nov 14, 2023
74 checks passed
@derrabus derrabus deleted the improvement/allow-other-exception-converters branch November 14, 2023 11:52
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants