Skip to content

Conversation

@Ace-Tang
Copy link
Contributor

since these functions like GetDevID, GetNodeIP ..., they all related
to the host system info, just move them into one file, and easy to
controll.

Signed-off-by: Ace-Tang [email protected]

Ⅰ. Describe what this PR did

since these functions like GetDevID, GetNodeIP ..., they all related
to the host system info, just move them into one file, and easy to
controll.

Ⅱ. Does this pull request fix one issue?

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

since these functions like GetDevID, GetNodeIP ..., they all related
to the host system info, just move them into one file, and easy to
controll.

Signed-off-by: Ace-Tang <[email protected]>
@codecov
Copy link

codecov bot commented Oct 30, 2018

Codecov Report

Merging #2390 into master will increase coverage by 0.02%.
The diff coverage is 70.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2390      +/-   ##
==========================================
+ Coverage   68.29%   68.32%   +0.02%     
==========================================
  Files         265      261       -4     
  Lines       18211    18211              
==========================================
+ Hits        12437    12442       +5     
+ Misses       4357     4351       -6     
- Partials     1417     1418       +1
Flag Coverage Δ
#criv1alpha1test 31.61% <37.93%> (-0.04%) ⬇️
#criv1alpha2test 35.47% <37.93%> (-0.04%) ⬇️
#integrationtest 39.58% <70.68%> (+0.02%) ⬆️
#nodee2etest 33.11% <37.93%> (-0.01%) ⬇️
#unittest 25.52% <0%> (ø) ⬆️
Impacted Files Coverage Δ
pkg/system/system.go 70.68% <70.68%> (ø)
cri/v1alpha2/cri_wrapper.go 62.4% <0%> (-1.21%) ⬇️
cri/v1alpha2/cri.go 68.31% <0%> (-0.49%) ⬇️
cri/v1alpha1/cri.go 60.7% <0%> (-0.33%) ⬇️
daemon/mgr/container.go 59.39% <0%> (ø) ⬆️
ctrd/container.go 59.76% <0%> (+0.95%) ⬆️
ctrd/image.go 78.94% <0%> (+2.19%) ⬆️
daemon/containerio/cri_log_file.go 88.23% <0%> (+3.92%) ⬆️
daemon/mgr/snapshot.go 94.2% <0%> (+4.34%) ⬆️

@Ace-Tang
Copy link
Contributor Author

Want to resolve sysinfo name discuss in #2271

Copy link
Contributor

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM

@fuweid fuweid merged commit 9bf53d1 into AliyunContainerService:master Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants