-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Stop swallowing exceptions #8962
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
greg0ire
force-pushed
the
dont-swallow-exceptions
branch
from
August 28, 2021 15:55
26bd70d
to
9240659
Compare
This was probably done in order to get rid of exceptions about tables already existing, but may and does swallow other exceptions as well, for instance exceptions about sequences failing to be created on Oracle because the identifier is too long. This makes it unnecessarily hard to understand what is going on.
greg0ire
force-pushed
the
dont-swallow-exceptions
branch
from
August 28, 2021 16:03
9240659
to
96b4c76
Compare
derrabus
approved these changes
Aug 29, 2021
This was referenced Sep 21, 2021
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. It looks like I based my grep on the comment inside. Today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated.
2 tasks
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. It looks like I based my grep on the comment inside. Today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated.
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. It looks like I based my grep on the comment inside. Today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated.
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. It looks like I based my grep on the comment inside. Today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated.
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. It looks like I based my grep on the comment inside. Today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated.
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. It looks like I based my grep on the comment inside. Today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated.
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. It looks like I based my grep on the comment inside. Today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated.
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. It looks like I based my grep on the comment inside. Today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated.
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. It looks like I based my grep on the comment inside. Today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated. Also, it turns out that swallowing exceptions is really the way to go here, because of the performance implication of calling dropSchema(). It is possible to catch a more precise exception though, which should preserve the benefits of not swallowing them.
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. It looks like I based my grep on the comment inside. Today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated. Also, it turns out that swallowing exceptions is really the way to go here, because of the performance implication of calling dropSchema(). It is possible to catch a more precise exception though, which should preserve the benefits of not swallowing them.
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. It looks like I based my grep on the comment inside. Today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated. Also, it turns out that swallowing exceptions is really the way to go here, because of the performance implication of calling dropSchema(). It is possible to catch a more precise exception though, which should preserve the benefits of not swallowing them.
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. As it turns out however, swallowing exceptions is really the way to go here, because of the performance implication of calling dropSchema(). It is possible to catch a more precise exception as well, which should preserve the benefits of not swallowing them. It looks like I based my grep on the comment inside the catch and today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated.
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 19, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. As it turns out however, swallowing exceptions is really the way to go here, because of the performance implication of calling dropSchema(). It is possible to catch a more precise exception as well, which should preserve the benefits of not swallowing them. It looks like I based my grep on the comment inside the catch and today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated.
greg0ire
added a commit
to greg0ire/doctrine-orm
that referenced
this pull request
Feb 20, 2022
In doctrine#8962, I established that swallowing exceptions while creating model was bad because it could hide other helpful exceptions. As it turns out however, swallowing exceptions is really the way to go here, because of the performance implication of calling dropSchema(). It is possible to catch a more precise exception as well, which should preserve the benefits of not swallowing them. It looks like I based my grep on the comment inside the catch and today, I found many other occurrences of this pattern, without the easy to grep comment. I decided to fix them as well, but in a lazier way: one no longer has to remember to call dropSchema in tearDown() now, it is automated.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was probably done in order to get rid of exceptions about tables
already existing, but may and does swallow other exceptions as well, for
instance exceptions about sequences failing to be created on Oracle
because the identifier is too long. This makes it unnecessarily hard to
understand what is going on.