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.

Sign up for GitHub

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

Jump to bottom

Fix linter errors #1379

Open
wants to merge 93 commits into
base: master
Choose a base branch
from
Open

Fix linter errors #1379

wants to merge 93 commits into from

Conversation

suhasagg
Copy link
Contributor

  • I added unit tests for any code that added
  • I updated the CHANGELOG.md
  • All IP is original and not copied from another source
  • I assign all copyright to Loom Network for the code in the pull request

@mukuls9971 mukuls9971 self-assigned this Jul 30, 2019
enlight
enlight requested changes Jul 30, 2019
View reviewed changes
auth/auth_test.go Outdated
@@ -72,6 +72,7 @@ func TestSignatureTxMiddlewareMultipleTxSameBlock(t *testing.T) {
//State is reset on every run
ctx2 := context.WithValue(context.Background(), ContextKeyOrigin, origin)
state2 := loomchain.NewStoreState(ctx2, store.NewMemStore(), abci.Header{Height: 27}, nil, nil)
//nolint: ineffassign
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't ignore these warnings, fix them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually assignment was assigning some variables which might be useful later on in tests but were not currently being used. I have fixed it as they can be assigned if needed.

eth/polls/polls_test.go Outdated
@@ -80,6 +81,7 @@ func testLogPoll(t *testing.T, version handler.ReceiptHandlerVersion) {
require.Equal(t, 0, len(logs.EthBlockLogs), "wrong number of logs returned")
state60 := common.MockStateAt(state, uint64(60))
sub.Remove(id)
//nolint: ineffassign
Copy link
Contributor

Choose a reason for hiding this comment

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

These too should be fixed, not ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually assignment was assigning some variables which might be useful later on in tests but were not currently being used. I have fixed it as they can be assigned if needed.

evm/loomethdb.go Outdated
params LogParams
batch batch
params LogParams
//nolint: unused, varcheck, deadcode, structcheck
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably fix all those

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

karma/karma.go Outdated Show resolved Hide resolved
privval/hsm/yubi.go Show resolved Hide resolved
store/multi_writer_app_store_test.go Outdated
@@ -146,6 +146,7 @@ func (m *MultiWriterAppStoreTestSuite) TestMultiWriterAppStoreSnapShotFlushInter
require.Equal([]byte("test2"), snapshotv1.Get([]byte("test2")))

// this flushes all data to disk
//nolint: ineffassign
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

throttle/contract_tx_limiter_middleware_test.go Outdated
addr3 = loom.MustParseAddress("default:0x5cecd1f7261e1f4c684e297be3edf03b825e01c5")
addr4 = loom.MustParseAddress("default:0x5cecd1f7261e1f4c684e297be3edf03b825e01c7")
addr5 = loom.MustParseAddress("default:0x5cecd1f7261e1f4c684e297be3edf03b825e01c9")
//nolint: unused, varcheck, deadcode
Copy link
Contributor

Choose a reason for hiding this comment

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

If these are really unused then remove them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@suhasagg suhasagg self-assigned this Jul 30, 2019
suhasagg added 6 commits July 30, 2019 14:15
@suhasagg
Use raw strings to avoid having to quote the backslashes.
1850210 
Solves ==>
should use raw string (`...`) with regexp.MustCompile to avoid having to escape twice (gosimple)
@suhasagg
Use raw strings to avoid having to quote the backslashes.
b9c3429 
Solves ==>
should use raw string (`...`) with regexp.MustCompile to avoid having to escape twice (gosimple)
@suhasagg
Remove unused variables
34b4a88
@suhasagg
Use raw strings to avoid having to quote the backslashes.
c1dbed3 
Solves ==>
should use raw string (`...`) with regexp.MustCompile to avoid having to escape twice (gosimple)
@suhasagg
Fix ineffassign
7e1838f
@suhasagg
fix ineffassign
4659b96
@suhasagg
Copy link
Contributor Author

suhasagg commented Jul 30, 2019 •
edited
Loading 

govet linter reporting a copied mutex
-----------------------------------------------------

evm/loomethdb.go:126:29: Dump passes lock by value: log.Logger contains sync.Mutex (govet)
func (b *batch) Dump(logger log.Logger) {
                            ^
evm/loomethdb.go:225:16: call of b.batch.Dump copies lock value: log.Logger contains sync.Mutex (govet)
		b.batch.Dump(logger)
		             ^
evm/loomethdb.go:244:16: call of b.batch.Dump copies lock value: log.Logger contains sync.Mutex (govet)
		b.batch.Dump(logger)
		             ^
evm/loomethdb.go:249:16: call of b.batch.Dump copies lock value: log.Logger contains sync.Mutex (govet)
		b.batch.Dump(logger)

@suhasagg
Copy link
Contributor Author

Some discussion in this context -
golang/go#13675

suhasagg and others added 25 commits August 1, 2019 19:53
@suhasagg
Fix ineffassign
3668f4c
@suhasagg
fix ineffassign
42b2e86
@suhasagg
call of b.batch.Dump copies lock value: log.Logger contains sync.Mutex
84acd0e 
Use pointer to logger
@suhasagg
Fix a bug
c018837
@suhasagg
Suppress go sec linter ==> false positive
0c1196e
@mukuls9971 @suhasagg
refactoring
a59f678
@suhasagg
Merged if condition
d790cbe
@suhasagg
Removed ineffective assignment
4529455
@suhasagg
Fix line length error
07d5cc1
@suhasagg
Fix line length error
128121d
@suhasagg
Remove ineffective assignment to version
440b3f2
@suhasagg
Fix line length error
fd24dff
@suhasagg
Fix line length error
dfd9f4d
@suhasagg
Fix line length error
eca1427
@suhasagg
Fix line length error
b88ce5c
@suhasagg
Fix Line Length error
69722b2
@suhasagg
Fix line length error
6bd7382
@suhasagg
Fix line length error
55ecc2f
@suhasagg
Fix line length error
e8ec615
@suhasagg
Fix line length errors
1cfd4cf
@suhasagg
Fix line length error
debb950
@suhasagg
Fix line length errors
b85e48c
@suhasagg
Merge branch 'Fix_Linter_Errors' of github.com:loomnetwork/loomchain …
4a9ec2a 
…into Fix_Linter_Errors
@suhasagg
Fix nolint warning
9a6558d
@suhasagg
Disable linting for full file - karma.go
228eea8
@traceCall traceCall requested a review from enlight August 8, 2019 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enlight enlight Awaiting requested review from enlight

Requested changes must be addressed to merge this pull request.

Assignees

@suhasagg suhasagg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@suhasagg @enlight @mukuls9971 @pathornteng