-
Notifications
You must be signed in to change notification settings - Fork 1.7k
skaffold init and buildpacks: skip dependencies #3758
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report
|
Fixes GoogleContainerTools#3568 Signed-off-by: David Gageot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but we should log something to the users since the node_modules and vendor packages are not unusual to be found.
@@ -32,17 +32,27 @@ func TestValidate(t *testing.T) { | |||
}{ | |||
{ | |||
description: "NodeJS", | |||
path: "path/to/package.json", | |||
path: filepath.Join("path", "to", "package.json"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just for completeness add corner test cases for
go.mod
, package.json
-> i.e. no filepath.Base = empty string
and somedir/vendor/go.mod
-> i.e vendor not first parent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. just few nits on more test cases
Signed-off-by: David Gageot <[email protected]>
name := filepath.Base(path) | ||
switch filepath.Base(path) { | ||
case "package.json": | ||
return !hasParent(path, "node_modules") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about using filepath.Match instead?
Fixes #3568
Signed-off-by: David Gageot [email protected]