feat: enable revive linter on the pbts branch #1907
Conversation
TestGenPrivKeySecp256k1
TestClientReplacesPrimaryWithWitnessIfPrimaryIsUnavailable
execAnsible function
This reverts commit ba108c6.
This is cherry-picked from: * #1907 There were a number of opportunities to simplify code in comet by using revive's if-return and early-return linters. This PR does not enable the linters as default, but instead just applies the fixes suggested by them. There are two cases where I've marked conditionals with //nolint -- they're both a little bit odd and it may make sense to look deeper into weather it is possible to benefit from early returns. As we've seen in the break statements in #1907 using a linter can help to enforce good practices across the entire repository and uncover issues. ## inspired by * #2156 --- #### PR checklist - [x] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --------- Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
This is cherry-picked from: * #1907 There were a number of opportunities to simplify code in comet by using revive's if-return and early-return linters. This PR does not enable the linters as default, but instead just applies the fixes suggested by them. There are two cases where I've marked conditionals with //nolint -- they're both a little bit odd and it may make sense to look deeper into weather it is possible to benefit from early returns. As we've seen in the break statements in #1907 using a linter can help to enforce good practices across the entire repository and uncover issues. ## inspired by * #2156 --- #### PR checklist - [x] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --------- Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> (cherry picked from commit eda385c) # Conflicts: # test/e2e/pkg/infra/docker/docker.go
|
FYI, we're waiting for PBTS https://github.com/cometbft/cometbft/tree/feature/pbts to land on |
There was a problem hiding this comment.
PBTS is a feature branch. The way we use these branches is that only changes relevant to the feature being worked on should land directly from a PR.
The rest should be reviewed and committed to main. And, via the periodic merges from main into the feature branch that we do, we will eventually get those changes. But, by planning the moment of those merges, we choose when those changes are to land on the feature branch.
|
OK, figure I should close this @sergio-mena ? I was basically trying to reduce future diffs. could just keep merging main to the other one. |
|
@faddat yes, please close this PR. I understand you are introducing the same changes on |
|
@sergio-mena the reason for this PR is that @melekes let me know that we were waiting for pbts to hit main. I will close it for now, it can be reopened later if need be. Could be a good reference later. |
This PR uses revive in largely the same manner as ibc-go does, and is targeted at the pbst branch.
It also became a great illustration of test flakiness:
In the course of writing this PR, all of the above tests were identified to be flaky. We don't know if that is the test, or the code it is testing, please keep in mind.
important
please don't merge or attempt to review this before merging #1906.
PR checklist
.changelog(we use unclog to manage our changelog)docs/orspec/) and code comments