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

build: enable goimports and varcheck linters #16446

Merged
merged 17 commits into from
Apr 17, 2018
Merged

build: enable goimports and varcheck linters #16446

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
ebf6df0
#15473 - add lint task to makefile
thomasmodeneis Apr 5, 2018
e79ce54
Merge branch 'master' of github.com:modeneis/go-ethereum
thomasmodeneis Apr 5, 2018
8889632
add gofmt, misspell, goconst, unconvert, gosimple, deadline=10m, min-…
thomasmodeneis Apr 5, 2018
c41a7cc
fixing all linting errs
thomasmodeneis Apr 5, 2018
9671bfc
add varcheck
thomasmodeneis Apr 5, 2018
a1b426a
Merge branch 'master' of github.com:ethereum/go-ethereum
thomasmodeneis Apr 5, 2018
a48f2a4
Merge branch 'master' of github.com:ethereum/go-ethereum
thomasmodeneis Apr 5, 2018
dea717b
goimports incorrectly removed valid imports, manually restoring all f…
thomasmodeneis Apr 5, 2018
b2e72fc
build: increasing deadline as travis build fails with error: (deadlin…
thomasmodeneis Apr 5, 2018
f77e0b4
build: revert the changes to import lists
thomasmodeneis Apr 6, 2018
84c3ead
build: gofmt all files
thomasmodeneis Apr 6, 2018
431af6b
build: run standard goimports on all files
thomasmodeneis Apr 6, 2018
ab0dcec
build: update script goimports+gofmt
thomasmodeneis Apr 6, 2018
639a37d
build: remove vendorcheck
thomasmodeneis Apr 9, 2018
db5d83a
build: remove vendorcheck
thomasmodeneis Apr 10, 2018
10ad506
build: remove deadline on lint job
thomasmodeneis Apr 10, 2018
fe98d8c
discv5: generate nodeevent
thomasmodeneis Apr 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
build: update script goimports+gofmt
  • Loading branch information
@thomasmodeneis
thomasmodeneis committed Apr 6, 2018
commit ab0dcec27d917445851a275f7e0c5896d8a0dd82
5 changes: 3 additions & 2 deletions build/goimports.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ find_files() {
}

GOFMT="gofmt -s -w";
GOFMT="goimports -w";
find_files | xargs $GOFMT;
GOIMPORTS="goimports -w";
find_files | xargs $GOFMT;
find_files | xargs $GOIMPORTS;
Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One of the reasons of linting errors was due to having code not properly formatted and not properly imported. There was no standard set for that. Now there is.

Copy link
Member

Choose a reason for hiding this comment

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

goimports is pretty much the standard tool used by any Go development environment. I don't really see the value of providing an extra utility method in our repo that is covered by the developer's editor anyway. I completely agree that the linter should enforce it, but its the developer's job to configure their dev environment, not ours.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I started to work on this issue, I found that the devs are not really doing what you said is their job...

consensus/consensus.go:1::warning: file is not goimported (goimports)
swarm/fuse/fuse_dir.go:1::warning: file is not goimported (goimports)
swarm/fuse/swarmfs.go:1::warning: file is not goimported (goimports)

So I ran the goimports the way "I think" is the best way, splitting the imports in 3. Just to find out that this is not the proffered way here.
Also, running goimports ./.. throws some errors, that I'm not sure how to fix, or even if they are really errors, so I added the script:

goimports -w ./..
../go-ethereum/crypto/bn256/bn256_fast.go:26:9: expected type, found '='
../go-ethereum/crypto/bn256/bn256_fast.go:30:9: expected type, found '='
../go-ethereum/crypto/bn256/bn256_fast.go:34:2: expected declaration, found 'return'
../go-ethereum/crypto/bn256/bn256_slow.go:26:9: expected type, found '='
../go-ethereum/crypto/bn256/bn256_slow.go:30:9: expected type, found '='
../go-ethereum/crypto/bn256/bn256_slow.go:34:2: expected declaration, found 'return'

So I added a "default" script to get this sorted and leave no room for errors.