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

add @throws declarations to docs in Connection #2810

Merged

Conversation

bestform
Copy link
Contributor

@bestform bestform commented Aug 3, 2017

In a few cases methods in Connection may throw a DBALException which
isn't declared in the docblock. As the Connection class is heavily
used by third party code, this code runs the risk of ignoring
those exceptions instead of handling them. While it is
technically not required to declare exceptions in PHP, it is highly
recommended, especially in public APIs.

Fixes #2809

In a few cases methods in Connection may throw a DBALException which
isn't declared in the docblock. As the Connection class is heavily
used by third party code, this code runs the risk of ignoring
those exceptions instead of handling them. While it is
technically not required to declare exceptions in PHP, it is highly
recommended, especially in public APIs.
@bestform
Copy link
Contributor Author

bestform commented Aug 3, 2017

To be honest I don't really understand the failure condition this patch is meeting in scrutinizer. Is a changed doc comment a failure condition?

@bestform
Copy link
Contributor Author

bestform commented Aug 3, 2017

Oh, never mind. I see. There are other unrelated errors in doc comments. My patch doesn't fix those - as it shouldn't - which leads to the error in scrutinizer.

@@ -410,6 +412,8 @@ private function detectDatabasePlatform()
* version without having to query it (performance reasons).
*
* @return string|null
*
* @throws \Exception
Copy link
Member

Choose a reason for hiding this comment

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

Exception is already imported

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. Fixed.

@Ocramius Ocramius self-assigned this Aug 3, 2017
@Ocramius Ocramius added this to the 2.7 milestone Aug 3, 2017
@Ocramius
Copy link
Member

Ocramius commented Aug 3, 2017

@bestform thanks!

@Ocramius Ocramius merged commit 408161b into doctrine:master Aug 3, 2017
@bestform
Copy link
Contributor Author

bestform commented Aug 3, 2017

anytime. Thanks for the merge.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2022
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