Skip to content

Conversation

AkihiroSuda
Copy link
Member

Signed-off-by: Akihiro Suda suda.akihiro@lab.ntt.co.jp

The runtime_root option in config.toml was not propagated to the shim.

Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
@AkihiroSuda AkihiroSuda force-pushed the fix-shim-runtime-root branch from 163f5c7 to faf2781 Compare February 28, 2018 05:15
@AkihiroSuda
Copy link
Member Author

Definitely we should have testsuite against several config.tomls (in another PR)

@AkihiroSuda AkihiroSuda added this to the 1.0.3 milestone Feb 28, 2018
@codecov-io
Copy link

Codecov Report

Merging #2173 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2173   +/-   ##
=======================================
  Coverage   41.11%   41.11%           
=======================================
  Files          66       66           
  Lines        7543     7543           
=======================================
  Hits         3101     3101           
  Misses       3961     3961           
  Partials      481      481
Flag Coverage Δ
#windows 41.11% <ø> (ø) ⬆️

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 f334749...faf2781. Read the comment docs.

@crosbymichael
Copy link
Member

LGTM

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit b3a4e63 into containerd:master Feb 28, 2018
@mlaventure
Copy link
Contributor

Should this has been done to ShimLocal too?

In which case I would have moved the logic to shimConfig() but @estesp was faster than my comment skills :)

@AkihiroSuda
Copy link
Member Author

Hmm, probably yes, thank you for pointing out.

I'll try to look into ShimLocal and tests as well, and then open a cherry-pick PR.

@AkihiroSuda
Copy link
Member Author

AkihiroSuda commented Mar 2, 2018

@mlaventure

I opened #2182

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants