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

bugfix: do not use string to convert int64 #2557

Merged
merged 1 commit into from
Dec 13, 2018
Merged

bugfix: do not use string to convert int64 #2557

merged 1 commit into from
Dec 13, 2018

Conversation

fuweid
Copy link
Contributor

@fuweid fuweid commented Dec 13, 2018

Signed-off-by: Wei Fu fhfuwei@163.com

Ⅰ. Describe what this PR did

We should use imageSize.String() instead of string(imageSize).
Otherwise, we will get messy display.

Ⅱ. Does this pull request fix one issue?

NONE

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

No need.

Ⅳ. Describe how to verify it

before fix:

➜  pouch git:(bugfix_image_list_display_issue) sudo pouch images
IMAGE ID       IMAGE NAME                                       SIZE
59788edf1f3e   registry.hub.docker.com/library/busybox:latest   򲸜

after fix:

➜  pouch git:(bugfix_image_list_display_issue) sudo pouch images
IMAGE ID       IMAGE NAME                                       SIZE
59788edf1f3e   registry.hub.docker.com/library/busybox:latest   715.53 KB

Ⅴ. Special notes for reviews

We should use imageSize.String() instead of string(imageSize).
Otherwise, we will get messy display.

Signed-off-by: Wei Fu <fhfuwei@163.com>
@codecov
Copy link

codecov bot commented Dec 13, 2018

Codecov Report

Merging #2557 into master will increase coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2557      +/-   ##
=========================================
+ Coverage   69.03%   69.1%   +0.06%     
=========================================
  Files         278     278              
  Lines       18581   18581              
=========================================
+ Hits        12828   12841      +13     
+ Misses       4272    4271       -1     
+ Partials     1481    1469      -12
Flag Coverage Δ
#criv1alpha1test 31.09% <ø> (-0.18%) ⬇️
#criv1alpha2test 35.59% <ø> (+0.14%) ⬆️
#integrationtest 40.56% <ø> (-0.02%) ⬇️
#nodee2etest 32.7% <ø> (+0.21%) ⬆️
#unittest 26.79% <ø> (ø) ⬆️
Impacted Files Coverage Δ
pkg/streams/utils.go 82.14% <0%> (-7.15%) ⬇️
ctrd/image.go 76.75% <0%> (-2.2%) ⬇️
daemon/logger/jsonfile/utils.go 71.54% <0%> (-1.63%) ⬇️
cri/v1alpha1/cri.go 60.26% <0%> (-0.67%) ⬇️
ctrd/container.go 59.2% <0%> (+0.79%) ⬆️
cri/v1alpha2/cri_wrapper.go 64.4% <0%> (+1.2%) ⬆️
cri/v1alpha2/cri.go 69.44% <0%> (+1.71%) ⬆️
daemon/mgr/snapshot.go 94.2% <0%> (+4.34%) ⬆️
cri/ocicni/cni_manager.go 70.58% <0%> (+11.76%) ⬆️

@pouchrobot pouchrobot added kind/bug This is bug report for project size/XS labels Dec 13, 2018
@fuweid fuweid requested review from ZYecho and zhuangqh December 13, 2018 07:09
@fengzixu
Copy link
Contributor

It is so interesting problem. :)

@fuweid
Copy link
Contributor Author

fuweid commented Dec 13, 2018

I was thinking that it is bug in golang.

@ZYecho
Copy link
Contributor

ZYecho commented Dec 13, 2018

LGTM wait for CI pass

@zhuangqh
Copy link
Contributor

LGTM

@zhuangqh
Copy link
Contributor

Converting a signed or unsigned integer value to a string type yields a string containing the UTF-8 representation of the integer. Values outside the range of valid Unicode code points are converted to "\uFFFD".

https://golang.org/ref/spec#Conversions search subtitle "Conversions to and from a string type"

@fuweid fuweid merged commit a340d95 into AliyunContainerService:master Dec 13, 2018
@fuweid fuweid deleted the bugfix_image_list_display_issue branch February 21, 2019 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug This is bug report for project size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants