Do not try to recover from panics, but use the return values #133
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
|
That is fair enough :) just fix the linter issues and i'll merge. |
|
Thanks for all the PRs, by the way. I'm just a bit busy right now, but i'll try to follow all of them. |
|
The linter issue is fixed by #132 I think the linter updated as I didn't tough that code... |
|
There are other linter issues in ResultsTable.go:814 and ConnectionsTable.go:77 |
svanharmelen
force-pushed
the
feat/errors
branch
from
6152825 to
f5053ea
Compare
|
Ah sorry, missed that one! Should be fixed now... |
svanharmelen
force-pushed
the
feat/errors
branch
from
f5053ea to
2af5d02
Compare
svanharmelen
force-pushed
the
feat/errors
branch
from
2af5d02 to
5fb2ffe
Compare
|
Addressed all feedback and rebased all PRs so they pass the linter again 👍🏻 |
Kuchteq
pushed a commit
to Kuchteq/lazysql
that referenced
this pull request
Nov 25, 2024
Do not try to recover from panics, but use the return values
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.
Sorry for all the PRs, I'm just having fun with it and am hitting a lot of small issues when using
lazysqlwhich I would like to help improve 😉This PR removes all the calls to
recoveras it is very discouraged (see Go docs) to try and recover panics as in in 99.999999% of the cases that indicates a bug in the program or another error not being handled properly which should be fixed instead.In order to take any actions using a
deferthat will only do something if the function returned an error, its best to just check against that error. While in Go its no too common to use named return values in the function signature, in the case where you want a defer to have access to the actual returned value I found it to be the most robust to at least name the error in the function signature. But in this case I noticed you already named all return values so I didn't need to change anything for that.