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.
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
refactor: replace panic with error in rules #1126
refactor: replace panic with error in rules #1126
Changes from 4 commits
89322bc
5665063
cf72c3d
689e3c9
3109bd6
c68d908
b607ec6
f5e3a0e
5c98306
63a0088
c7f09e0
a53dfc7
d47112b
fd97ee3
a451c02
1c6e8e7
f17c9b1
ae29706
fdcaa68
af86a92
0022083
9c9e05e
92bcf9e
aee44cb
2d6cafa
ea832f4
d6f17ff
3086a32
093971c
d78563b
5685fad
68e556a
3191797
f6c7ee0
0e4a8c4
ef674f6
5ebe459
953b2a5
ed5e149
1628b5c
96de931
bad6ed8
634fa48
e56fa62
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't simply add a new return argument to the
Apply
function due to the backward compatibility. Someone who usesrevivelib
may depend on this interface.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok @alexandear any idea how to handle errors without of course excuting panic(), we can use in each Apply:
but it is not elegant way :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To maintain backwards compatibility we would have to add backwards compatible wrappers around the newly included changes. And one thing that came to my mind, how do these changes fit the use of
Revive
ingolangci-lint
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok got it @denisvmedia you want something like that: https://github.com/golangci/golangci-lint/blob/master/pkg/golinters/revive/revive.go#L53 ? but is it possible to do that in separate PR, or it should be done in that PR?