Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linters: enable stylecheck #4153

Merged
merged 4 commits into from
Nov 16, 2019
Merged

linters: enable stylecheck #4153

merged 4 commits into from
Nov 16, 2019

Conversation

melekes
Copy link
Contributor

@melekes melekes commented Nov 16, 2019

#3961

This PR repairs linter errors seen when running the following command:
golangci-lint run --no-config --disable-all=true --enable=stylecheck

Contributes to #3262

  • Referenced an issue explaining the need for the change
  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG_PENDING.md

var _, err1 = cdc.MarshalBinaryLengthPrefixedWriter(conn, locEphPub)
if err1 != nil {
return nil, err1, true // abort
return nil, false, err1 // abort
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!!!!!!!!!!!!!!!!!!!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: create an issue because tests weren't failing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@melekes melekes added T:breaking Type: Breaking Change R:major PR contains breaking changes that have to wait till a major release is made to be merged labels Nov 16, 2019
@melekes melekes self-assigned this Nov 16, 2019
@codecov-io
Copy link

Codecov Report

Merging #4153 into master will increase coverage by 0.01%.
The diff coverage is 58.75%.

@@            Coverage Diff             @@
##           master    #4153      +/-   ##
==========================================
+ Coverage   66.68%   66.69%   +0.01%     
==========================================
  Files         247      247              
  Lines       21221    21221              
==========================================
+ Hits        14151    14154       +3     
+ Misses       6010     6008       -2     
+ Partials     1060     1059       -1
Impacted Files Coverage Δ
crypto/encoding/amino/amino.go 100% <ø> (ø) ⬆️
mempool/errors.go 25% <ø> (ø) ⬆️
rpc/grpc/api.go 88.88% <ø> (ø) ⬆️
state/txindex/indexer.go 75% <ø> (ø) ⬆️
rpc/grpc/client_server.go 81.81% <ø> (ø) ⬆️
cmd/tendermint/commands/root.go 51.72% <0%> (ø) ⬆️
libs/common/bit_array.go 80.08% <0%> (ø) ⬆️
crypto/merkle/simple_proof.go 71.66% <0%> (ø) ⬆️
libs/log/filter.go 71.79% <0%> (ø) ⬆️
rpc/core/mempool.go 0% <0%> (ø) ⬆️
... and 66 more

Copy link
Contributor

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@melekes melekes merged commit 3e1516b into master Nov 16, 2019
@melekes melekes deleted the go-green/stylecheck branch November 16, 2019 15:35
@@ -45,6 +45,7 @@ program](https://hackerone.com/tendermint).
- Go API
- [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) `Query#(Matches|Conditions)` returns an error.
- [rpc/client] \#3471 `Validators` now requires two more args: `page` and `perPage`
- [libs/common] \#3262 Make error the last parameter of `Task` (@PSalant726)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also cryptoAmino rename.

Also I thought I saw some XYZError get renamed to ErrXYZ but I can't find it now and not sure it was even in a package covered by our versioning

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also cryptoAmino rename.

how's this breaking? I am not even sure it's worth mentioning in the changelog

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't it a change in package name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo it's only breaking if you rename the folder

Client code uses the package path when importing the package. By convention, the last element of the package path is the package name

https://blog.golang.org/package-names

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R:major PR contains breaking changes that have to wait till a major release is made to be merged T:breaking Type: Breaking Change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants