Skip to content

getEngineConfigFilePath is only used during test…#1304

Merged
silvin-lubecki merged 1 commit into
docker:masterfrom
vdemeester:move-test-function-in-there
Aug 22, 2018
Merged

getEngineConfigFilePath is only used during test…#1304
silvin-lubecki merged 1 commit into
docker:masterfrom
vdemeester:move-test-function-in-there

Conversation

@vdemeester
Copy link
Copy Markdown
Collaborator

… so moving it in test files for now.

Signed-off-by: Vincent Demeester vincent@sbr.pm

… so moving it in test files for now.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #1304 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1304      +/-   ##
==========================================
- Coverage   54.76%   54.73%   -0.04%     
==========================================
  Files         292      292              
  Lines       19285    19267      -18     
==========================================
- Hits        10561    10545      -16     
+ Misses       8064     8063       -1     
+ Partials      660      659       -1

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM ❤️

}

// getEngineConfigFilePath will extract the config file location from the engine flags
func (c baseClient) getEngineConfigFilePath(ctx context.Context, engine containerd.Container) (string, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Originally this was in use, but in the refactorings it wasn't necessary. My suspicion is we may need this again as things are fleshed out further in the future, but we can pull it back into the main package at that time.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes exactly 😉

Copy link
Copy Markdown
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

@silvin-lubecki silvin-lubecki merged commit 022fd9b into docker:master Aug 22, 2018
@GordonTheTurtle GordonTheTurtle added this to the 18.09.0 milestone Aug 22, 2018
@silvin-lubecki silvin-lubecki deleted the move-test-function-in-there branch August 22, 2018 15:08
@thaJeztah thaJeztah modified the milestones: 18.09.0, 19.03.0 Sep 14, 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.

6 participants