Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

Check for errors when trying to create the auth token file #447

Merged
merged 3 commits into from
Mar 24, 2017
Merged

Check for errors when trying to create the auth token file #447

merged 3 commits into from
Mar 24, 2017

Conversation

danielfm
Copy link
Contributor

@danielfm danielfm commented Mar 23, 2017

@mumoshu addressing your comment in my last PR #439.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 23, 2017
@codecov-io
Copy link

codecov-io commented Mar 23, 2017

Codecov Report

Merging #447 into master will decrease coverage by <.01%.
The diff coverage is 50%.

@@            Coverage Diff             @@
##           master     #447      +/-   ##
==========================================
- Coverage   41.02%   41.01%   -0.01%     
==========================================
  Files          38       38              
  Lines        2679     2682       +3     
==========================================
+ Hits         1099     1100       +1     
- Misses       1420     1421       +1     
- Partials      160      161       +1
Impacted Files Coverage Δ
core/controlplane/config/token_config.go 69.56% <50%> (-0.98%) ⬇️

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 1f08f3b...fd84a23. Read the comment docs.

@danielfm danielfm changed the title Check for errors when trying to create the auth token file [WIP} Check for errors when trying to create the auth token file Mar 23, 2017
@danielfm danielfm changed the title [WIP} Check for errors when trying to create the auth token file [WIP] Check for errors when trying to create the auth token file Mar 23, 2017
if err != nil {
return nil, err
}
defer file.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

you never use file, why defer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@redbaron I just wanted to make sure all handles were closed properly. Is this unnecessary in this case?

Copy link
Contributor

Choose a reason for hiding this comment

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

defer delays file.Close() until function exits. you don't use file anywhere, so close it right there, without defer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@redbaron done! Thanks for your feedback.

The file is not being used after it's created/opened, so there's no
reason to defer the closing.
@danielfm danielfm changed the title [WIP] Check for errors when trying to create the auth token file Check for errors when trying to create the auth token file Mar 23, 2017
@mumoshu mumoshu merged commit 263135a into kubernetes-retired:master Mar 24, 2017
@mumoshu
Copy link
Contributor

mumoshu commented Mar 24, 2017

LGTM 👍
Thanks for contributing @danielfm, and for reviewing @redbaron 🙇

@danielfm danielfm deleted the token-file-creation-error-check branch March 24, 2017 00:11
kylehodgetts pushed a commit to HotelsDotCom/kube-aws that referenced this pull request Mar 27, 2018
…s-retired#447)

* Check for errors when trying to create the auth token file
* Close the file right away, instead of closing with `defer`

The file is not being used after it's created/opened, so there's no
reason to defer the closing.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants