review nits + walk->analyze

pkg/skaffold/initializer/analyze.go

@@ -29,67 +29,65 @@ import (
 )
 
 type analysis struct {
-	kubectlAnalyzer  *KubectlAnalyzer
-	skaffoldAnalyzer *SkaffoldConfigAnalyzer
-	builderAnalyzer  *BuilderAnalyzer
+	kubectlAnalyzer  *kubectlAnalyzer
+	skaffoldAnalyzer *skaffoldConfigAnalyzer
+	builderAnalyzer  *builderAnalyzer
 }
 
-// Analyzer is a generic Visitor that is called on every file in the directory
+// analyzer is a generic Visitor that is called on every file in the directory
 // It can manage state and react to walking events assuming a bread first search
-type Analyzer interface {
-	EnterDir(dir string)
-	AnalyzeFile(file string) error
-	ExitDir(dir string)
+type analyzer interface {
+	enterDir(dir string)
+	analyzeFile(file string) error
+	exitDir(dir string)
 }
 
-type DirectoryAnalyzer struct {
+type directoryAnalyzer struct {
 	currentDir string
 }
 
-func (a *DirectoryAnalyzer) AnalyzeFile(filePath string) error {
+func (a *directoryAnalyzer) analyzeFile(filePath string) error {
 	return nil
 }
 
-func (a *DirectoryAnalyzer) EnterDir(dir string) {
+func (a *directoryAnalyzer) enterDir(dir string) {
 	a.currentDir = dir
 }
-func (a *DirectoryAnalyzer) ExitDir(dir string) {
+
+func (a *directoryAnalyzer) exitDir(dir string) {
 	//pass
 }
 
-type KubectlAnalyzer struct {
-	DirectoryAnalyzer
+type kubectlAnalyzer struct {
+	directoryAnalyzer
 	kubernetesManifests []string
 }
 
-func (a *KubectlAnalyzer) AnalyzeFile(filePath string) error {
-	isSkaffoldConfig := IsSkaffoldConfig(filePath)
-	isKubernetesManifest := kubectl.IsKubernetesManifest(filePath)
-
-	if isKubernetesManifest && !isSkaffoldConfig {
+func (a *kubectlAnalyzer) analyzeFile(filePath string) error {
+	if kubectl.IsKubernetesManifest(filePath) && !IsSkaffoldConfig(filePath) {
 		a.kubernetesManifests = append(a.kubernetesManifests, filePath)
 	}
 	return nil
 }
 
-type SkaffoldConfigAnalyzer struct {
-	DirectoryAnalyzer
+type skaffoldConfigAnalyzer struct {
+	directoryAnalyzer
 	force bool
 }
 
-func (a *SkaffoldConfigAnalyzer) AnalyzeFile(filePath string) error {
-	isSkaffoldConfig := IsSkaffoldConfig(filePath)
-	if isSkaffoldConfig {
-		if !a.force {
-			return fmt.Errorf("pre-existing %s found (you may continue with --force)", filePath)
-		}
-		logrus.Debugf("%s is a valid skaffold configuration: continuing since --force=true", filePath)
+func (a *skaffoldConfigAnalyzer) analyzeFile(filePath string) error {
+	if !IsSkaffoldConfig(filePath) {
+		return nil
+	}
+	if !a.force {
+		return fmt.Errorf("pre-existing %s found (you may continue with --force)", filePath)
 	}
+	logrus.Debugf("%s is a valid skaffold configuration: continuing since --force=true", filePath)
 	return nil
 }
 
-type BuilderAnalyzer struct {
-	DirectoryAnalyzer
+type builderAnalyzer struct {
+	directoryAnalyzer
 	enableJibInit       bool
 	enableBuildpackInit bool
 	findBuilders        bool
@@ -98,7 +96,7 @@ type BuilderAnalyzer struct {
 	parentDirToStopFindBuilders string
 }
 
-func (a *BuilderAnalyzer) AnalyzeFile(filePath string) error {
+func (a *builderAnalyzer) analyzeFile(filePath string) error {
 	if a.findBuilders && (a.parentDirToStopFindBuilders == "" || a.parentDirToStopFindBuilders == a.currentDir) {
 		builderConfigs, continueSearchingBuilders := detectBuilders(a.enableJibInit, a.enableBuildpackInit, filePath)
 		a.foundBuilders = append(a.foundBuilders, builderConfigs...)
@@ -109,67 +107,62 @@ func (a *BuilderAnalyzer) AnalyzeFile(filePath string) error {
 	return nil
 }
 
-func (a *BuilderAnalyzer) ExitDir(dir string) {
+func (a *builderAnalyzer) exitDir(dir string) {
 	if a.parentDirToStopFindBuilders == dir {
 		a.parentDirToStopFindBuilders = ""
 	}
 }
 
-// walk recursively walks a directory and returns the k8s configs and builder configs that it finds
-func (a *analysis) walk(root string) error {
-	var analyze func(dir string) error
-	analyze = func(dir string) error {
-		for _, analyzer := range a.analyzers() {
-			analyzer.EnterDir(dir)
-		}
-		dirents, err := godirwalk.ReadDirents(dir, nil)
-		if err != nil {
-			return err
-		}
+// analyze recursively walks a directory and returns the k8s configs and builder configs that it finds
+func (a *analysis) analyze(dir string) error {
+	for _, analyzer := range a.analyzers() {
+		analyzer.enterDir(dir)
+	}
+	dirents, err := godirwalk.ReadDirents(dir, nil)
+	if err != nil {
+		return err
+	}
 
-		var subdirectories []*godirwalk.Dirent
-		//this is for deterministic results - given the same directory structure
-		//init should have the same results
-		sort.Sort(dirents)
+	var subdirectories []*godirwalk.Dirent
+	//this is for deterministic results - given the same directory structure
+	//init should have the same results
+	sort.Sort(dirents)
 
-		// Traverse files
-		for _, file := range dirents {
-			if util.IsHiddenFile(file.Name()) || util.IsHiddenDir(file.Name()) {
-				continue
-			}
-
-			// If we found a directory, keep track of it until we've gone through all the files first
-			if file.IsDir() {
-				subdirectories = append(subdirectories, file)
-				continue
-			}
+	// Traverse files
+	for _, file := range dirents {
+		if util.IsHiddenFile(file.Name()) || util.IsHiddenDir(file.Name()) {
+			continue
+		}
 
-			filePath := filepath.Join(dir, file.Name())
-			for _, analyzer := range a.analyzers() {
-				if err := analyzer.AnalyzeFile(filePath); err != nil {
-					return err
-				}
-			}
+		// If we found a directory, keep track of it until we've gone through all the files first
+		if file.IsDir() {
+			subdirectories = append(subdirectories, file)
+			continue
 		}
 
-		// Recurse into subdirectories
-		for _, subdir := range subdirectories {
-			if err = analyze(filepath.Join(dir, subdir.Name())); err != nil {
+		filePath := filepath.Join(dir, file.Name())
+		for _, analyzer := range a.analyzers() {
+			if err := analyzer.analyzeFile(filePath); err != nil {
 				return err
 			}
 		}
+	}
 
-		for _, analyzer := range a.analyzers() {
-			analyzer.ExitDir(dir)
+	// Recurse into subdirectories
+	for _, subdir := range subdirectories {
+		if err = a.analyze(filepath.Join(dir, subdir.Name())); err != nil {
+			return err
 		}
-		return nil
 	}
 
-	return analyze(root)
+	for _, analyzer := range a.analyzers() {
+		analyzer.exitDir(dir)
+	}
+	return nil
 }
 
-func (a *analysis) analyzers() []Analyzer {
-	return []Analyzer{
+func (a *analysis) analyzers() []analyzer {
+	return []analyzer{
 		a.kubectlAnalyzer,
 		a.skaffoldAnalyzer,
 		a.builderAnalyzer,

pkg/skaffold/initializer/config_test.go

@@ -22,9 +22,7 @@ import (
 	"testing"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/buildpacks"
-
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
-
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )

pkg/skaffold/initializer/init.go

@@ -94,18 +94,18 @@ func DoInit(ctx context.Context, out io.Writer, c Config) error {
 	}
 
 	a := &analysis{
-		kubectlAnalyzer: &KubectlAnalyzer{},
-		builderAnalyzer: &BuilderAnalyzer{
+		kubectlAnalyzer: &kubectlAnalyzer{},
+		builderAnalyzer: &builderAnalyzer{
 			findBuilders:        !c.SkipBuild,
 			enableJibInit:       c.EnableJibInit,
 			enableBuildpackInit: c.EnableBuildpackInit,
 		},
-		skaffoldAnalyzer: &SkaffoldConfigAnalyzer{
+		skaffoldAnalyzer: &skaffoldConfigAnalyzer{
 			force: c.Force,
 		},
 	}
 
-	if err := a.walk(rootDir); err != nil {
+	if err := a.analyze(rootDir); err != nil {
 		return err
 	}
 

pkg/skaffold/initializer/init_test.go

@@ -331,17 +331,17 @@ deploy:
 			t.Override(&jib.Validate, fakeValidateJibConfig)
 
 			a := &analysis{
-				kubectlAnalyzer: &KubectlAnalyzer{},
-				builderAnalyzer: &BuilderAnalyzer{
+				kubectlAnalyzer: &kubectlAnalyzer{},
+				builderAnalyzer: &builderAnalyzer{
 					findBuilders:        true,
 					enableJibInit:       test.enableJibInit,
 					enableBuildpackInit: test.enableBuildpackInit,
 				},
-				skaffoldAnalyzer: &SkaffoldConfigAnalyzer{
+				skaffoldAnalyzer: &skaffoldConfigAnalyzer{
 					force: test.force,
 				},
 			}
-			err := a.walk(tmpDir.Root())
+			err := a.analyze(tmpDir.Root())
 
 			t.CheckError(test.shouldErr, err)
 			if test.shouldErr {