A bit of cleanup based on goconst tool #8529
Conversation
| @@ -14,6 +14,8 @@ import ( | |||
| "google.golang.org/grpc/status" | |||
| ) | |||
|
|
|||
| const epochError = "Cannot retrieve information about an epoch in the future, current epoch %d, requesting %d" | |||
There was a problem hiding this comment.
Can you please change all ...Error to err...? E.g. errEpoch. This is the convention we use. Also, this name should be more expressive: errNoEpochInfo, just like in beacon-chain/rpc/beacon/validators_test.go.
| @@ -22,7 +22,7 @@ import ( | |||
| "github.com/prysmaticlabs/prysm/validator/keymanager/derived" | |||
| ) | |||
|
|
|||
| var ( | |||
| const ( | |||
There was a problem hiding this comment.
The exact same mnemonic repeats in 3 places. I think it warrants moving to a shared package.
There was a problem hiding this comment.
I thought so as well but didn't come up with any good place. Is there any preference where should I move this?
There was a problem hiding this comment.
I think you can create a new constants.go file inside validator/testing.
| @@ -32,6 +32,10 @@ import ( | |||
| "github.com/prysmaticlabs/prysm/shared/timeutils" | |||
| ) | |||
|
|
|||
| const ( | |||
| noEpochInfoError = "Cannot retrieve information about an epoch in the future" | |||
There was a problem hiding this comment.
| noEpochInfoError = "Cannot retrieve information about an epoch in the future" | |
| noFutureEpochInfoError = "Cannot retrieve information about an epoch in the future" |
There was a problem hiding this comment.
I tend to step away from suggestion comments for names. Committing this suggestion will break the build because a proper refactor won't happen.
Just my observation :-)
|
I am afraid adding a new files requires some Bazel related configuration, because the CI is failing. Any tips would be appreciated :) validator/testing/BUILD.bazel 1970-01-01 00:00:00.000000000 +0000 | 0s
-- | --
| @@ -4,6 +4,7 @@
| name = "go_default_library",
| testonly = True,
| srcs = [
| + "constants.go",
| "mock_protector.go",
| "mock_slasher.go",
| "protection_history.go",
| FAIL: Gazelle needs to be run
| Please run bazel run //:gazelle -- fix |
|
Hi Panagiotis, you can run `bazel run //:gazelle -- fix` to resolve your
issue
…On Mon, Mar 1, 2021 at 11:19 AM Panagiotis Georgiadis < ***@***.***> wrote:
I am afraid adding a new files requires some Bazel related configuration,
because the CI is failing. Any tips would be appreciated :)
validator/testing/BUILD.bazel 1970-01-01 00:00:00.000000000 +0000 | 0s
-- | --
| @@ -4,6 +4,7 @@
| name = "go_default_library",
| testonly = True,
| srcs = [
| + "constants.go",
| "mock_protector.go",
| "mock_slasher.go",
| "protection_history.go",
| FAIL: Gazelle needs to be run
| Please run bazel run //:gazelle -- fix
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#8529 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABKQQPIXHN2K56VIQKQMQZLTBPEALANCNFSM4YK7QHHQ>
.
--
*Raul E. Jordan | Co-Founder, Prysmatic Labs*
prysmaticlabs.com
rauljordan.com
Twitter: @raulitojordan <https://twitter.com/raulitojordan>
gpg --receive-keys 95452A701810FEDB
|
|
cc @drpaneas |
Alright, many thanks for the blazing fast reply :D I did run this. Let's see if CI likes it this time :) |
Codecov Report
@@ Coverage Diff @@
## develop #8529 +/- ##
===========================================
+ Coverage 58.19% 58.21% +0.01%
===========================================
Files 461 461
Lines 33100 33100
===========================================
+ Hits 19264 19268 +4
+ Misses 10910 10907 -3
+ Partials 2926 2925 -1 |
What type of PR is this?
What does this PR do? Why is it needed?
Make the project one step closer to be compliant with
goconstwhich is part of golangciWhich issues(s) does this PR fix?
None
Other notes for review
If you prefer 1 PR == 1 commit, let me know so I can squash the commits.