Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

Warn about implicit contextDir change #117

Merged
merged 2 commits into from
Jul 11, 2016
Merged

Warn about implicit contextDir change #117

merged 2 commits into from
Jul 11, 2016

Conversation

ctrlok
Copy link
Contributor

@ctrlok ctrlok commented Jul 10, 2016

© @ybogdanov

Why is the default behavior taking base dir of Rockerfile instead of a current working directory? Because this behavior is more intuitive than the opposite. You can run a Rockerfile from any directory while keeping its functionality without a need to specify the context directory repetitively. Docker solves the latter by making the last argument mandatory.
But yes, it's a good idea of making a warning if a base directory of Rockerfile is not same as current and the last argument is not specified explicitly.

@@ -296,6 +296,8 @@ func buildCommand(c *cli.Context) {
if !filepath.IsAbs(contextDir) {
contextDir = filepath.Join(wd, args[0])
}
} else if contextDir != wd {
log.Warningf("Default context directory is %s. You can specify context direcotry as the last argument.", contextDir)
Copy link
Contributor

Choose a reason for hiding this comment

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

@ctrlok
direcotry -> directory

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say something like this:

log.Warningf("Implicit context directory used: %s. You can override context directory using the last argument.", contextDir)

@ybogdanov ybogdanov changed the title add warning for default contextDir assigment Warn about implicit contextDir change Jul 11, 2016
@ctrlok
Copy link
Contributor Author

ctrlok commented Jul 11, 2016

@ybogdanov @hitthefrei Done.

@ybogdanov ybogdanov merged commit 350156f into dev Jul 11, 2016
@ybogdanov ybogdanov deleted the f-contextDir-warning branch July 25, 2016 13:53
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.

3 participants