🧹 Linting

[faddat]

Description

This PR updates the linters so that we no longer use deprecated linters, enforces linting in CI, and fixes linter errors.

closes: #1417

Please note that this changes a number of important names, but likely most important:

every time we named something with Id, that has been changed to ID per revive.

---

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.

• Targeted PR against correct branch (see CONTRIBUTING.md)
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the module structure standards.
• Wrote unit and integration tests
• Updated relevant documentation (docs/) or specification (x/<module>/spec/)
• Added relevant godoc comments.
• Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer
• Review Codecov Report in the comment section below once CI passes

[codecov-commenter]

Codecov Report

Merging #1418 (db496c2) into main (8987888) will decrease coverage by 0.07%.
The diff coverage is 76.05%.

@@            Coverage Diff             @@
##             main    #1418      +/-   ##
==========================================
- Coverage   80.05%   79.97%   -0.08%     
==========================================
  Files         164      165       +1     
  Lines       12300    12367      +67     
==========================================
+ Hits         9847     9891      +44     
- Misses       1993     2012      +19     
- Partials      460      464       +4     

Impacted Files	Coverage Δ
modules/apps/27-interchain-accounts/module.go	54.87% <0.00%> (-2.82%)	⬇️
modules/apps/29-fee/module.go	53.57% <0.00%> (-1.99%)	⬇️
modules/apps/transfer/keeper/relay.go	88.11% <ø> (ø)
modules/core/02-client/keeper/client.go	98.22% <ø> (ø)
modules/core/02-client/keeper/grpc_query.go	68.20% <ø> (ø)
modules/core/02-client/keeper/proposal.go	86.36% <ø> (ø)
modules/core/03-connection/keeper/handshake.go	89.83% <ø> (-0.06%)	⬇️
modules/core/03-connection/keeper/keeper.go	90.43% <ø> (ø)
modules/core/04-channel/types/packet.go	67.56% <0.00%> (ø)
modules/core/keeper/msg_server.go	58.29% <ø> (ø)
... and 28 more

[faddat]

The first run failed due to timeout, so I gave it a longer timeout.

[crodriguezvega]

Thanks for these improvements, @faddat!

I think it would also make sense to add a brief explanation of the API breaking changes in the migration docs. If you prefer, we can take care of that ourselves in a separate PR.

[AdityaSripal]

Thanks Jacob, I'll defer to the implementation team here. I just have a couple of questions/suggestions

[faddat]

Note: some of these defy normal convention in Cosmos. What I would like to do is come up with a standard set of linters that we can use for SDK 46 onwards, so that we end up not triggering liter errors and needing to use golint. I started work on that over at Osmosis, and eventually hope to apply that work to the Cosmos SDK.

[faddat]

I suspect they'll perpetually be grouped with stdlib, but I'll kick that off now.

[colin-axner]

LGTM, only minor concerns left:

• e2e module naming change (think this is likely fine as is)
• RedundancyDecorator renamed to RedundantRelayDecorator? Did we want to do this change?

[damiannolan]

LGTM, only minor concerns left:

• e2e module naming change (think this is likely fine as is)
• RedundancyDecorator renamed to RedundantRelayDecorator? Did we want to do this change?

Do you think we could additionally add an exception to ignore .pb.go files for goimports ordering? Generally these should be read-only afaik! cc. @faddat

Thanks for all your work on getting this over the line!

[faddat]

@vuong177 is re-doing protos now

make proto-all doesn't work on my computer for some reason

[faddat]

RedundancyDecorator renamed to RedundantRelayDecorator? Did we want to do this change?

-- I don't think so?

I'll have a look and try to figure out how it got in because I don't recall making it. maybe it triggered the "stutters" thing? checking...

[colin-axner]

RedundancyDecorator renamed to RedundantRelayDecorator? Did we want to do this change?

That seemed to be the conclusion to me, but I don't have a strong preference. It just seems like a more descriptive name. Did it cause an issue with the linter?

[colin-axner]

Great work!! Thanks for following up on all our suggestions. I'm still curious about the Decorator naming, but don't want to block merge on that discussion

[crodriguezvega]

@faddat There seem to be some conflicts...

[faddat]

@colin-axner let's keep the name as it is, then. It doesn't cause an issue with the linter. In fact, once this is merged, our editors should be able to scream at us about even the most trivial lint issues. It's 100% golang-ci lint'd.