sync: coreth PR #1279: style: forbidigo t.Fatal #1849
Conversation
JonathanOppenheimer
changed the title
Signed-off-by: Jonathan Oppenheimer <[email protected]>
| } | ||
| default: | ||
| t.Fatalf("unknown step: %d", step) | ||
| require.Failf(t, "unknown step", "got: %d", step) |
There was a problem hiding this comment.
This is an overreaching linter that replaces stdlib features with something that does nothing more than require the user to know that it's the same thing.
There was a problem hiding this comment.
We need this linter to be added for the monorepo goal. Is there a better way of doing this? There could be a nolint clause instead
alarso16
left a comment
alarso16
left a comment
There was a problem hiding this comment.
The point of the linter is to use the helper functions. require.Fail should be avoided whenever possible
|
What's with the goleak failures? It's weird because the |
Co-authored-by: Austin Larson <[email protected]> Signed-off-by: Jonathan Oppenheimer <[email protected]>
alarso16
left a comment
alarso16
left a comment
There was a problem hiding this comment.
We should fix the licensing on this file and exclude these changes first
JonathanOppenheimer
force-pushed
the
JonathanOppenheimer/sync-pr-1279-forbidgo
branch
from
52bb4cd to
25db6a9
Compare
Done -- see #1865 |
|
This blocked by this PR |
Signed-off-by: Jonathan Oppenheimer <[email protected]>
| # - pattern: require\.Error$(# ErrorIs should be used instead)? | ||
| # - pattern: require\.ErrorContains$(# ErrorIs should be used instead)? | ||
| # - pattern: require\.EqualValues$(# Equal should be used instead)? | ||
| # - pattern: require\.NotEqualValues$(# NotEqual should be used instead)? | ||
| - pattern: ^(t|b|tb|f)\.(Fatal|Fatalf|Error|Errorf)$(# the require library should be used instead)? | ||
| - pattern: ^sort\.(Slice|Strings)$(# the slices package should be used instead)? | ||
| # - pattern: ^sort\.(Slice|Strings)$(# the slices package should be used instead)? |
There was a problem hiding this comment.
why these are commented out?
There was a problem hiding this comment.
These are commented out as to not make this PR overly large (and it's already a massive PR).
These linters will be added in a seperate PR.
Co-authored-by: Ceyhun Onur <[email protected]> Signed-off-by: Jonathan Oppenheimer <[email protected]>
| // the next upgradeBytes. (only check the result on the last apply) | ||
| if i != len(tt.configs)-1 { | ||
| require.NoError(t, err, "expecting checkConfigCompatible call %d to return nil", i+1) | ||
| require.Nil(t, err, "expecting checkConfigCompatible call %d to return nil", i+1) |
There was a problem hiding this comment.
These actually have to be nil
Signed-off-by: Jonathan Oppenheimer <[email protected]>
Syncs ava-labs/coreth#1279.
Note: The first commit 96fc992 is a perfect cherry pick of the coreth changes. These should not be reviewed stringently -- if you have changes, make them to coreth first~
The second commit 1a0ecea contains subnet-evm specific changes and should be reviewed as normal
--