core: fix remaining golangci-lint issues #10972
Conversation
|
I see that you haven't updated any CHANGELOG files. Would it make sense to do so? |
jmank88
force-pushed
the
golangci-lint-2023-10-16
branch
from
2a94340 to
70b02c1
Compare
| @@ -525,6 +525,8 @@ func NewJobResource(j job.Job) *JobResource { | |||
| resource.BootstrapSpec = NewBootstrapSpec(j.BootstrapSpec) | |||
| case job.Gateway: | |||
| resource.GatewaySpec = NewGatewaySpec(j.GatewaySpec) | |||
| case job.LegacyGasStationServer, job.LegacyGasStationSidecar: | |||
| // unsupported | |||
There was a problem hiding this comment.
Is this OK? Or an oversight to correct?
There was a problem hiding this comment.
There was a problem hiding this comment.
Removing this should be OK, functionality is equivalent right?
There was a problem hiding this comment.
Yeah this change aims to be equivalent while satisfying the linter.
But was it an oversight that support was not added in the first place? Should we create a ticket?
There was a problem hiding this comment.
I think it was intentional that it's unsupported. cc @jinhoonbang
There was a problem hiding this comment.
@jinhoonbang is an owner of this GasStation I believe ;)
| @@ -304,7 +305,7 @@ func TestLegacyMode(t *testing.T) { | |||
| require.Equal(t, true, tm.legacyMode) | |||
| require.Len(t, tm.endpoints, 1) | |||
|
|
|||
| clientSent := make([]synchronization.TelemPayload, 0) | |||
| var clientSent []synchronization.TelemPayload | |||
There was a problem hiding this comment.
The linter understands the length check down below when we use the simpler declaration style 🤷
| type broadcasterHelperCfg struct { | ||
| highestSeenHead *evmtypes.Head | ||
| db *sqlx.DB | ||
| } |
There was a problem hiding this comment.
This was awkward and barely used any longer, so I simplified it away. The db field was no cost, but the tradeoff for highestSeenHead is to pass some nils instead 🤷
There was a problem hiding this comment.
i don't love required args with nil values, but the surface is small and only in tests so maybe ok.
l like XXXcfg.New() (*XXX) even less...
There was a problem hiding this comment.
Yeah I thought the same but didn't stress since it's not even exported
jmank88
requested review from
jkongie,
a team,
connorwstein,
samsondav,
PiotrTrzpil,
archseer,
cfal,
reductionista and
prashantkumar1982
as code owners
jmank88
force-pushed
the
golangci-lint-2023-10-16
branch
from
70b02c1 to
bc8ae73
Compare
|
Currently debugging one test, and a rebase flagged ~60 new issues 🤦 cc @chainchad it seems like the PR linter is still broken/out-of-sync with overnight & local runs. |
jmank88
force-pushed
the
golangci-lint-2023-10-16
branch
3 times, most recently
from
fa4aa5f to
de438d9
Compare
jmank88
force-pushed
the
golangci-lint-2023-10-16
branch
4 times, most recently
from
a811e2d to
eeb30cb
Compare
jmank88
force-pushed
the
golangci-lint-2023-10-16
branch
from
eeb30cb to
dbe0f0c
Compare
|
SonarQube Quality Gate
|
This PR cleans up the last of the remaining linter issues.