Skip to content

Lint: Tackle Reported Lint Issues #2541

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

Closed
mohammed90 opened this issue Mar 30, 2019 · 10 comments
Closed

Lint: Tackle Reported Lint Issues #2541

mohammed90 opened this issue Mar 30, 2019 · 10 comments
Labels
feature ⚙️ New feature or request good first issue 🐤 Good for newcomers

Comments

@mohammed90
Copy link
Member

1. What would you like to have changed?

Clean, successful output of golangci-lint run -E gofmt -E goimports -E misspell against Caddy's repo with ~0 issues reported.

2. Why is this feature a useful, necessary, and/or important addition to this project?

Code hygiene, correctness, and visibility of possible failures.

3. What alternatives are there, or what are you doing in the meantime to work around the lack of this feature?

🤷‍♂️

4. Please link to any relevant issues, pull requests, or other discussions.

#2540

5. How to start

  • Install golangci-lint with go get -v github.com/golangci/golangci-lint/cmd/golangci-lint
  • Clone caddy's repo
  • Run golangci-lint run -E gofmt -E goimports -E misspell against the repo
  • Go wild
@mohammed90 mohammed90 added the feature ⚙️ New feature or request label Mar 30, 2019
@mholt mholt added the good first issue 🐤 Good for newcomers label Mar 30, 2019
u5surf added a commit to u5surf/caddy that referenced this issue Mar 31, 2019
u5surf added a commit to u5surf/caddy that referenced this issue Mar 31, 2019
u5surf added a commit to u5surf/caddy that referenced this issue Mar 31, 2019
u5surf added a commit to u5surf/caddy that referenced this issue Mar 31, 2019
u5surf added a commit to u5surf/caddy that referenced this issue Apr 1, 2019
u5surf added a commit to u5surf/caddy that referenced this issue Apr 1, 2019
u5surf added a commit to u5surf/caddy that referenced this issue Apr 1, 2019
u5surf added a commit to u5surf/caddy that referenced this issue Apr 1, 2019
@u5surf
Copy link
Contributor

u5surf commented Apr 1, 2019

@mohammed90 @mholt
I tried to this issue and fix gofmt, gosimple, deadcode, unused and serveral error codes of staticcheck which is SA6002, SA6004, SA6005, SA9004, SA1009
And remaining staticcheck is following

$ golangci-lint run -E gofmt -E goimports -E misspell -D errcheck
caddyhttp/proxy/reverseproxy.go:607:3: SA1019: t.Dial is deprecated: Use DialContext instead, which allows the transport to cancel dials as soon as they are no longer needed. If both are set, DialContext takes priority.  (staticcheck)
		t.Dial = b.Dial
		^
caddyhttp/proxy/reverseproxy.go:617:2: SA1019: t.Dial is deprecated: Use DialContext instead, which allows the transport to cancel dials as soon as they are no longer needed. If both are set, DialContext takes priority.  (staticcheck)
	t.Dial = func(network, addr string) (net.Conn, error) {
	^
caddyhttp/proxy/reverseproxy.go:634:5: SA1019: t.Dial is deprecated: Use DialContext instead, which allows the transport to cancel dials as soon as they are no longer needed. If both are set, DialContext takes priority.  (staticcheck)
	if t.Dial != nil {
	   ^

because, net/http.Transport has not supported DialTLSContext which is DialContext compatible yet. golang/go#21526
And all errcheck fix remaining in progress.

Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 1, 2019
@Inconnu08
Copy link

Hello @mohammed90 @mholt,
Sorry, I'm new to contributing. I am trying to fix all lint errchecks. Would like to know if I'm going towards the right direction. Thank you.

@mohammed90
Copy link
Member Author

@Inconnu08 yes, you got those right. It seems like @u5surf is about to fix those as well, so maybe you two should coordinate.

@Inconnu08
Copy link

Oh sorry about that, @u5surf. I was only done with caddy_test.go, caddyfile/lexer_test.go and caddyhttp/basicauth/basicauth.go . Let me know if I could do some of them.

Also I just finished lint structchecks. @mohammed90

Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 1, 2019
@u5surf
Copy link
Contributor

u5surf commented Apr 1, 2019

@Inconnu08 Oh, I’m sorry I should have said the work in progress in advance 😅
In fact, I burnout to fix these lintings. So you can fix all of them which I remain. I think it is only remained errcheck.
@mohammed99 Let me tell you whether is enough to fix of linting which we commit.

@Inconnu08
Copy link

@u5surf it's okay. I'll take it from here.
@mohammed90 please assign me this. I would like to complete the rest.

@mholt
Copy link
Member

mholt commented Apr 2, 2019

@Inconnu08 You got it. (I can only assign collaborators.)

Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 2, 2019
Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 2, 2019
Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 2, 2019
Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 3, 2019
Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 3, 2019
Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 5, 2019
- reverseproxy
- reverseproxy_test
- upstream
- upstream_test
- body_test
Inconnu08 referenced this issue in Inconnu08/caddy Apr 6, 2019
- handler_test
- redirect_test
- requestid_test
- rewrite_test
- fileserver_test
Inconnu08 referenced this issue in Inconnu08/caddy Apr 6, 2019
- websocket
- setup
- collection
- redirect_test
- templates_test
@Inconnu08
Copy link

@mohammed90 @mholt done with all errchecks. Please let me know if any changes are needed.

@mohammed90
Copy link
Member Author

It's easier to review the changes if you submit the pull request. This will also allow for the CI to run against the changes and report any failures or lint issues.

Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 6, 2019
run goimports against caddyserver#2551
- lexer_test
- log_test
- markdown
Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 6, 2019
Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 6, 2019
Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 6, 2019
Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 6, 2019
@u5surf u5surf mentioned this issue Apr 8, 2019
Inconnu08 added a commit to Inconnu08/caddy that referenced this issue Apr 9, 2019
mholt pushed a commit that referenced this issue Apr 22, 2019
* Lint: fix some errcheck #2541

* Lint: fix passing structcheck #2541

* Lint: update fix structcheck #2541

* Lint: fix errcheck for basicauth, browse, fastcgi_test #2541

* Lint: fix errcheck for browse, fastcgi_test, fcgiclient, fcgiclient_test #2541

* Lint: fix errcheck for responsefilter_test, fcgilient_test #2541

* Lint: fix errcheck for header_test #2541

* Lint: update errcheck for fcgiclient_test #2541

* Lint: fix errcheck for server, header_test, fastcgi_test, https_test, recorder_test #2541

* Lint: fix errcheck for tplcontext, vhosttrie_test, internal_test, handler_test #2541

* Lint: fix errcheck for log_test, markdown mholt#2541

* Lint: fix errcheck for policy, body_test, proxy_test #2541

* Lint: fix errcheck for on multiple packages #2541

- reverseproxy
- reverseproxy_test
- upstream
- upstream_test
- body_test

* Lint: fix errcheck in multiple packages mholt#2541
- handler_test
- redirect_test
- requestid_test
- rewrite_test
- fileserver_test

* Lint: fix errcheck in multiple packages mholt#2541

- websocket
- setup
- collection
- redirect_test
- templates_test

* Lint: fix errcheck in logger test #2541

run goimports against #2551
- lexer_test
- log_test
- markdown

* Update caddyhttp/httpserver/logger_test.go

Co-Authored-By: Inconnu08 <[email protected]>

* Update log_test.go

* Lint: fix scope in logger_test #2541

* remove redundant err check in logger_test #2541

* fix alias in logger_test #2541

* fix import for format #2541

* refactor variable names and error check #2541
@mholt mholt closed this as completed Apr 22, 2019
@u5surf
Copy link
Contributor

u5surf commented Apr 23, 2019

@mholt @mohammed90 @Inconnu08
I fixed gofmt, gosimple, deadcode, unused and serveral error codes of staticcheck which is SA6002, SA6004, SA6005, SA9004, SA1009 in #2554
Did you review it? It seems not to do yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request good first issue 🐤 Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants