Skip to content

Fix gometalint errors #2840

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
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

Fix gometalint errors
da8db46 
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

@manishtomar manishtomar requested a review from dmcgowan February 5, 2019 00:31
@dmcgowan
Copy link
Collaborator

dmcgowan commented Feb 5, 2019

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

Remove err nil check
48818fd 
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
Reviewers

@dmcgowan dmcgowan Awaiting requested review from dmcgowan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@manishtomar @dmcgowan @B7478