Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

virtcontainers: fix codes misunderstanding in virtcontainers #326

Merged
merged 1 commit into from
May 21, 2018
Merged

virtcontainers: fix codes misunderstanding in virtcontainers #326

merged 1 commit into from
May 21, 2018

Conversation

jshachm
Copy link
Member

@jshachm jshachm commented May 19, 2018

Still there are some codes left which
will cause some misunderstanding

Change p in short of pod into s or sandbox

Fixes: #325

Signed-off-by: Haomin [email protected]

@katabuilder
Copy link

PSS Measurement:
Qemu: 144180 KB
Proxy: 4758 KB
Shim: 8812 KB

Memory inside container:
Total Memory: 2045972 KB
Free Memory: 2011092 KB

@codecov
Copy link

codecov bot commented May 19, 2018

Codecov Report

Merging #326 into master will not change coverage.
The diff coverage is 52.17%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #326   +/-   ##
=======================================
  Coverage   63.87%   63.87%           
=======================================
  Files          87       87           
  Lines        8620     8620           
=======================================
  Hits         5506     5506           
  Misses       2530     2530           
  Partials      584      584
Impacted Files Coverage Δ
virtcontainers/pkg/vcmock/sandbox.go 0% <0%> (ø) ⬆️
virtcontainers/sandbox.go 67.45% <100%> (ø) ⬆️
virtcontainers/api.go 63.12% <88.88%> (ø) ⬆️

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 2245e67...7abb8fe. Read the comment docs.

@@ -39,13 +39,13 @@ func CreateSandbox(sandboxConfig SandboxConfig) (VCSandbox, error) {
}

func createSandboxFromConfig(sandboxConfig SandboxConfig) (*Sandbox, error) {
// Create the sandbox.
// Create the s.
Copy link
Member

Choose a reason for hiding this comment

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

Please keep sandbox in the comment

s, err := createSandbox(sandboxConfig)
if err != nil {
return nil, err
}

// Create the sandbox network
// Create the s network
Copy link
Member

Choose a reason for hiding this comment

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

same here.

@@ -60,7 +60,7 @@ func createSandboxFromConfig(sandboxConfig SandboxConfig) (*Sandbox, error) {
return nil, err
}

// The sandbox is completely created now, we can store it.
// The s is completely created now, we can store it.
Copy link
Member

Choose a reason for hiding this comment

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

and here.

Copy link
Member Author

Choose a reason for hiding this comment

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

sry for that using refactor in Goland IDE and it did silly thing...

@katabuilder
Copy link

PSS Measurement:
Qemu: 146008 KB
Proxy: 6712 KB
Shim: 11074 KB

Memory inside container:
Total Memory: 2045972 KB
Free Memory: 2011092 KB

@@ -129,27 +129,27 @@ func StartSandbox(sandboxID string) (VCSandbox, error) {
defer unlockSandbox(lockFile)

// Fetch the sandbox from storage and create it.
p, err := fetchSandbox(sandboxID)
sandbox, err := fetchSandbox(sandboxID)
Copy link
Member

Choose a reason for hiding this comment

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

Please use s to be consistent with other places.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are still some sandbox not s in functions like StatusSandbox.May be I should change them to s

@bergwolf What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

@jshachm Yes, please. Thanks!

Still there are some codes left which
will cause some misunderstanding

Change `p` in short of `pod` into `s` or `sandbox`

Fixes: #325

Signed-off-by: Haomin <[email protected]>
@katabuilder
Copy link

PSS Measurement:
Qemu: 142178 KB
Proxy: 6781 KB
Shim: 9039 KB

Memory inside container:
Total Memory: 2045972 KB
Free Memory: 2011340 KB

Copy link
Member

@bergwolf bergwolf left a comment

Choose a reason for hiding this comment

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

Thanks @jshachm !

Copy link
Member

@caoruidong caoruidong left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@devimc devimc left a comment

Choose a reason for hiding this comment

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

lgtm

@caoruidong
Copy link
Member

Since this pr only includes rename, merge it regardless of codecov.

@caoruidong caoruidong merged commit 92ec15d into kata-containers:master May 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants