chore: address linter findings in 07-tendermint #6136
Conversation
Co-authored-by: DimitrisJim <[email protected]>
bznein
requested review from
colin-axner,
AdityaSripal,
damiannolan and
chatton
as code owners
WalkthroughThe recent updates across various light client modules in the IBC framework focus on enhancing type safety and error handling. These improvements include added checks for type assertion success and refined error management strategies, ensuring more robust and fault-tolerant client operations. Changes
Possibly related issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| var clientState *ClientState | ||
| clientState, ok := clientStateI.(*ClientState) | ||
| if !ok { | ||
| panic(fmt.Errorf("cannot convert %T into %T", clientStateI, clientState)) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| height := header.GetHeight().(clienttypes.Height) | ||
| height, ok := header.GetHeight().(clienttypes.Height) | ||
| if !ok { | ||
| panic(fmt.Errorf("cannot convert %T to %T", header.GetHeight(), &clienttypes.Height{})) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| var consensusState *ConsensusState | ||
| consensusState, ok := consensusStateI.(*ConsensusState) | ||
| if !ok { | ||
| panic(fmt.Errorf("cannot convert %T into %T", consensusStateI, consensusState)) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| var clientState *ClientState | ||
| clientState, ok := clientStateI.(*ClientState) | ||
| if !ok { | ||
| panic(fmt.Errorf("cannot convert %T into %T", clientStateI, clientState)) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
There was a problem hiding this comment.
lgtm, wondering if it might be best to wait for Charly's PR #6074 for moving these tests over to reduce conflicts there
There was a problem hiding this comment.
Pretty admirable your persistence to fix all these linter errors, man. :)
We will have some merge conflicts, or linter screaming again depending on whether this PR mergers first or the ones for moving the 07-tendermint tests for client state to the light client module (this and this). Something just to keep in mind...
Thanks! I'll keep this PR open and solve merge conflicts when those two gets merged :) |
There was a problem hiding this comment.
Thanks!
It will be nice when we eventually remove the exported.Height interface in favour of using the concrete type directly in all cases. This will remove all of these type conversions which are now in a lot of places.
The exported interface only exists due to historical reasons for avoiding import cycles within different sub pkgs in the codebase, and there is no good reason why there should exist multiple implementations of the Height type.
We have plans to remove to it soon ™️
Description
Addresses all linting issues that would be found in 07-tendermint by upgrading golanci-lint version (see linked issue)
ref: #6086
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/).godoccomments.Files changedin the GitHub PR explorer.SonarCloud Reportin the comment section below once CI passes.Summary by CodeRabbit