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

Fix gometalint errors #2840

Merged
merged 2 commits into from Feb 5, 2019
Merged

Fix gometalint errors #2840

merged 2 commits into from Feb 5, 2019

Conversation

manishtomar
Copy link
Contributor

Fix lint errors that occurred after recent change in gometalinter alecthomas/gometalinter#579

Signed-off-by: Manish Tomar <manish.tomar@docker.com>
@manishtomar
Copy link
Contributor Author

The only lint error is

$ make check
+ check
gometalinter --config .gometalinter.json ./...
registry/storage/garbagecollect.go:107:4:warning: when ok is true, err can't be nil (S1020) (staticcheck)
make: *** [check] Error 1

which I don't understand why since err is not checked after the type assertion: https://github.com/docker/distribution/blob/da8db4666b08431e701b044c9077648533d97b7e/registry/storage/garbagecollect.go#L107

@dmcgowan
Copy link
Collaborator

dmcgowan commented Feb 5, 2019

Try removing the err != nil check since the type check already covers that case

since type checking nil will not panic and return appropriately

Signed-off-by: Manish Tomar <manish.tomar@docker.com>
@codecov
Copy link

codecov bot commented Feb 5, 2019

Codecov Report

Merging #2840 into master will increase coverage by 0.3%.
The diff coverage is 73.68%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2840     +/-   ##
=========================================
+ Coverage   60.15%   60.45%   +0.3%     
=========================================
  Files         103      102      -1     
  Lines        8038     8001     -37     
=========================================
+ Hits         4835     4837      +2     
+ Misses       2560     2517     -43     
- Partials      643      647      +4
Flag Coverage Δ
#linux 60.45% <73.68%> (+0.3%) ⬆️
Impacted Files Coverage Δ
registry/storage/blobstore.go 59.78% <ø> (+3.08%) ⬆️
registry/storage/tagstore.go 78.49% <ø> (+8.3%) ⬆️
registry/api/v2/urls.go 76.52% <ø> (+3.18%) ⬆️
registry/storage/schema2manifesthandler.go 65.57% <ø> (ø) ⬆️
registry/proxy/proxyblobstore.go 55.45% <ø> (ø) ⬆️
notifications/bridge.go 54.31% <ø> (+2.24%) ⬆️
notifications/sinks.go 95% <ø> (ø) ⬆️
registry/handlers/blobupload.go 44.72% <0%> (ø) ⬆️
manifest/ocischema/builder.go 66.66% <0%> (ø) ⬆️
registry/handlers/hooks.go 0% <0%> (ø) ⬆️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b75069e...48818fd. Read the comment docs.

@dmcgowan
Copy link
Collaborator

dmcgowan commented Feb 5, 2019

LGTM

@dmcgowan dmcgowan merged commit 0d3efad into distribution:master Feb 5, 2019
@manishtomar manishtomar deleted the fix-lint branch February 5, 2019 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants