Format sort.go.

Change error messages. Remove unnecessary variable. Remove named returns. [#121903541] Signed-off-by: Danny Berger <dberger@pivotal.io>
cloudfoundry-attic · Jun 29, 2016 · 365cb4e · 365cb4e
1 parent 3de0905
commit 365cb4e
Showing 6 changed files with 70 additions and 71 deletions.
diff --git a/release/pkg/sort.go b/release/pkg/sort.go
@@ -2,12 +2,12 @@ package pkg
 
 import (
 	"errors"
+	"fmt"
 )
 
 // Topologically sorts an array of packages
-func Sort(releasePackages []*Package) (sortedPackages []*Package, err error){
-	err = nil
-	sortedPackages = []*Package{}
+func Sort(releasePackages []*Package) ([]*Package, error) {
+	sortedPackages := []*Package{}
 
 	incomingEdges, outgoingEdges := getEdgeMaps(releasePackages)
 	noIncomingEdgesSet := []*Package{}
@@ -17,8 +17,7 @@ func Sort(releasePackages []*Package) (sortedPackages []*Package, err error){
 			noIncomingEdgesSet = append(noIncomingEdgesSet, pkg)
 		}
 	}
-	size := len(noIncomingEdgesSet)
-	for size > 0 {
+	for len(noIncomingEdgesSet) > 0 {
 		elem := noIncomingEdgesSet[0]
 		noIncomingEdgesSet = noIncomingEdgesSet[1:]
 
@@ -30,28 +29,27 @@ func Sort(releasePackages []*Package) (sortedPackages []*Package, err error){
 				noIncomingEdgesSet = append(noIncomingEdgesSet, pkg)
 			}
 		}
-		size = len(noIncomingEdgesSet)
 	}
 	for _, edges := range incomingEdges {
 		if len(edges) > 0 {
-			err = errors.New("Circular dependency detected.")
+			return nil, errors.New("Circular dependency detected while sorting packages")
 		}
 	}
-	return
+	return sortedPackages, nil
 }
 
-func removeFromList(packageList []*Package, pkg *Package) []*Package{
+func removeFromList(packageList []*Package, pkg *Package) []*Package {
 	for idx, elem := range packageList {
 		if elem == pkg {
-			packageList = append(packageList[:idx], packageList[idx+1:]...)
+			return append(packageList[:idx], packageList[idx+1:]...)
 		}
 	}
-	return packageList
+	panic(fmt.Sprintf("Expected %s to be in dependency graph", pkg.Name))
 }
 
-func getEdgeMaps(releasePackages []*Package) (incomingEdges, outgoingEdges  map[*Package][]*Package){
-	incomingEdges = make(map[*Package][]*Package)
-	outgoingEdges = make(map[*Package][]*Package)
+func getEdgeMaps(releasePackages []*Package) (map[*Package][]*Package, map[*Package][]*Package) {
+	incomingEdges := make(map[*Package][]*Package)
+	outgoingEdges := make(map[*Package][]*Package)
 
 	for _, pkg := range releasePackages {
 		incomingEdges[pkg] = []*Package{}
@@ -65,5 +63,5 @@ func getEdgeMaps(releasePackages []*Package) (incomingEdges, outgoingEdges  map[
 			}
 		}
 	}
-	return
+	return incomingEdges, outgoingEdges
 }
diff --git a/release/pkg/sort_test.go b/release/pkg/sort_test.go
@@ -184,7 +184,7 @@ var _ = Describe("Sort", func() {
 		})
 	})
 
-	Context("graph with sibbling dependencies", func() {
+	Context("graph with sibling dependencies", func() {
 		var (
 			golang, runC, garden, guardian Package
 		)
@@ -193,17 +193,17 @@ var _ = Describe("Sort", func() {
 			golang = Package{Name: "golang"}
 
 			runC = Package{
-				Name: "runC",
+				Name:         "runC",
 				Dependencies: []*Package{&golang},
 			}
 
 			guardian = Package{
-				Name: "guardian",
+				Name:         "guardian",
 				Dependencies: []*Package{&runC, &golang},
 			}
 
 			garden = Package{
-				Name: "garden",
+				Name:         "garden",
 				Dependencies: []*Package{&guardian},
 			}
 
@@ -215,7 +215,7 @@ var _ = Describe("Sort", func() {
 			}
 		})
 
-		It("orders the packages as: golang, runC, guardian, garden", func(){
+		It("orders the packages as: golang, runC, guardian, garden", func() {
 			sortedPackages, _ := Sort(packages)
 
 			Expect(sortedPackages[0].Name).To(Equal(golang.Name))
@@ -225,23 +225,23 @@ var _ = Describe("Sort", func() {
 		})
 	})
 
-	Context("Graph with circular dependency", func(){
+	Context("Graph with circular dependency", func() {
 		var (
 			package1,
 			package2,
 			package3 Package
 		)
-		BeforeEach(func(){
+		BeforeEach(func() {
 			package1 = Package{
-				Name: "fake-package-name-1",
+				Name:         "fake-package-name-1",
 				Dependencies: []*Package{},
 			}
 			package2 = Package{
-				Name: "fake-package-name-2",
+				Name:         "fake-package-name-2",
 				Dependencies: []*Package{&package1},
 			}
 			package3 = Package{
-				Name: "fake-package-name-3",
+				Name:         "fake-package-name-3",
 				Dependencies: []*Package{&package2},
 			}
 
@@ -259,80 +259,80 @@ var _ = Describe("Sort", func() {
 		})
 	})
 
-	Context("Graph from a CPI release", func(){
-		var packages []*Package;
-		BeforeEach(func(){
-			pid_utils := Package{ Name: "pid_utils",}
+	Context("Graph from a CPI release", func() {
+		var packages []*Package
+		BeforeEach(func() {
+			pid_utils := Package{Name: "pid_utils"}
 
-			iptables := Package{ Name: "iptables",}
+			iptables := Package{Name: "iptables"}
 
-			bosh_io_release_resource := Package{ Name: "bosh_io_release_resource",}
+			bosh_io_release_resource := Package{Name: "bosh_io_release_resource"}
 
-			s3_resource := Package{ Name: "s3_resource",}
+			s3_resource := Package{Name: "s3_resource"}
 
-			bosh_io_stemcell_resource := Package{ Name: "bosh_io_stemcell_resource",}
+			bosh_io_stemcell_resource := Package{Name: "bosh_io_stemcell_resource"}
 
 			golang := Package{Name: "golang"}
 
 			baggageclaim := Package{
-				Name: "baggageclaim",
+				Name:         "baggageclaim",
 				Dependencies: []*Package{&golang},
 			}
 
 			jettison := Package{
-				Name: "jettison",
+				Name:         "jettison",
 				Dependencies: []*Package{&golang},
 			}
-			cf_resource := Package{ Name: "cf_resource",}
+			cf_resource := Package{Name: "cf_resource"}
 
-			golang_161 := Package{ Name: "golang_1.6.1",}
+			golang_161 := Package{Name: "golang_1.6.1"}
 
 			runc := Package{
-				Name: "runc",
+				Name:         "runc",
 				Dependencies: []*Package{&golang_161},
 			}
 
 			guardian := Package{
-				Name: "guardian",
+				Name:         "guardian",
 				Dependencies: []*Package{&golang_161, &runc},
 			}
 
-			btrfs_tools := Package{ Name: "btrfs_tools",}
+			btrfs_tools := Package{Name: "btrfs_tools"}
 
-			docker_image_resource := Package{ Name: "docker_image_resource",}
+			docker_image_resource := Package{Name: "docker_image_resource"}
 
 			resource_discovery := Package{
-				Name: "resource_discovery",
+				Name:         "resource_discovery",
 				Dependencies: []*Package{&golang},
 			}
 
-			github_release_resource := Package{ Name: "github_release_resource",}
+			github_release_resource := Package{Name: "github_release_resource"}
 
-			shadow := Package{ Name: "shadow",}
+			shadow := Package{Name: "shadow"}
 
-			vagrant_cloud_resource := Package{ Name: "vagrant_cloud_resource",}
+			vagrant_cloud_resource := Package{Name: "vagrant_cloud_resource"}
 
-			pool_resource := Package{ Name: "pool_resource",}
+			pool_resource := Package{Name: "pool_resource"}
 
-			bosh_deployment_resource := Package{ Name: "bosh_deployment_resource",}
+			bosh_deployment_resource := Package{Name: "bosh_deployment_resource"}
 
-			generated_worker_key := Package{ Name: "generated_worker_key",}
+			generated_worker_key := Package{Name: "generated_worker_key"}
 
-			archive_resource := Package{ Name: "archive_resource",}
+			archive_resource := Package{Name: "archive_resource"}
 
-			time_resource := Package{ Name: "time_resource",}
+			time_resource := Package{Name: "time_resource"}
 
-			git_resource := Package{ Name: "git_resource",}
+			git_resource := Package{Name: "git_resource"}
 
-			busybox := Package{ Name: "busybox",}
+			busybox := Package{Name: "busybox"}
 
-			semver_resource := Package{ Name: "semver_resource",}
+			semver_resource := Package{Name: "semver_resource"}
 
-			hg_resource := Package{ Name: "hg_resource",}
+			hg_resource := Package{Name: "hg_resource"}
 
-			tar := Package{ Name: "tar",}
+			tar := Package{Name: "tar"}
 
-			tracker_resource := Package{ Name: "tracker_resource",}
+			tracker_resource := Package{Name: "tracker_resource"}
 
 			packages = []*Package{
 				&bosh_io_release_resource,
@@ -366,7 +366,7 @@ var _ = Describe("Sort", func() {
 				&runc}
 		})
 
-		It("should sort the packages correctly", func(){
+		It("should sort the packages correctly", func() {
 			hasBeenLoaded := map[string]bool{}
 
 			for _, pkg := range packages {
@@ -378,7 +378,6 @@ var _ = Describe("Sort", func() {
 			for _, pkg := range sortedPackages {
 				if pkg.Dependencies != nil {
 					for _, dep := range pkg.Dependencies {
-						fmt.Printf("Expected %s dependency %s to have been loaded\n", pkg.Name, dep.Name)
 						Expect(hasBeenLoaded[dep.Name]).To(BeTrue())
 					}
 				}

diff --git a/state/job/dependency_compiler.go b/state/job/dependency_compiler.go
@@ -53,7 +53,7 @@ func (c *dependencyCompiler) Compile(releaseJobs []bireljob.Job, stage biui.Stag
 }
 
 // resolveJobPackageCompilationDependencies returns all packages required by all specified jobs, in compilation order (reverse dependency order)
-func (c *dependencyCompiler) resolveJobCompilationDependencies(releaseJobs []bireljob.Job) (sortedPackages []*birelpkg.Package, err error) {
+func (c *dependencyCompiler) resolveJobCompilationDependencies(releaseJobs []bireljob.Job) ([]*birelpkg.Package, error) {
 	// collect and de-dupe all required packages (dependencies of jobs)
 	packageMap := map[string]*birelpkg.Package{}
 	for _, releaseJob := range releaseJobs {
@@ -71,15 +71,18 @@ func (c *dependencyCompiler) resolveJobCompilationDependencies(releaseJobs []bir
 	}
 
 	// sort in compilation order
-	sortedPackages, err = birelpkg.Sort(packages)
+	sortedPackages, err := birelpkg.Sort(packages)
+	if err != nil {
+		return nil, err
+	}
 
 	pkgs := []string{}
 	for _, pkg := range sortedPackages {
 		pkgs = append(pkgs, fmt.Sprintf("%s/%s", pkg.Name, pkg.Fingerprint))
 	}
 	c.logger.Debug(c.logTag, "Sorted dependencies:\n%s", strings.Join(pkgs, "\n"))
 
-	return
+	return sortedPackages, nil
 }
 
 // resolvePackageDependencies adds the releasePackage's dependencies to the packageMap recursively

diff --git a/state/job/dependency_compiler_test.go b/state/job/dependency_compiler_test.go
@@ -139,23 +139,23 @@ var _ = Describe("DependencyCompiler", func() {
 		}))
 	})
 
-	Context("Graph with circular dependency", func(){
+	Context("Graph with circular dependency", func() {
 		var (
 			package1,
 			package2,
 			package3 *birelpkg.Package
 		)
-		BeforeEach(func(){
+		BeforeEach(func() {
 			package1 = &birelpkg.Package{
-				Name: "fake-package-name-1",
+				Name:         "fake-package-name-1",
 				Dependencies: []*birelpkg.Package{},
 			}
 			package2 = &birelpkg.Package{
-				Name: "fake-package-name-2",
+				Name:         "fake-package-name-2",
 				Dependencies: []*birelpkg.Package{package1},
 			}
 			package3 = &birelpkg.Package{
-				Name: "fake-package-name-3",
+				Name:         "fake-package-name-3",
 				Dependencies: []*birelpkg.Package{package2},
 			}
 

diff --git a/state/pkg/resolve_dependencies.go b/state/pkg/resolve_dependencies.go
@@ -1,7 +1,6 @@
 package pkg
 
 import (
-
 	birelpkg "github.com/cloudfoundry/bosh-init/release/pkg"
 )
 
@@ -51,8 +50,8 @@ func joinUnique(a []*birelpkg.Package, b []*birelpkg.Package) []*birelpkg.Packag
 }
 
 func reverse(a []*birelpkg.Package) []*birelpkg.Package {
-	for i := len(a)/2-1; i >= 0; i-- {
-		opp := len(a)-1-i
+	for i := len(a)/2 - 1; i >= 0; i-- {
+		opp := len(a) - 1 - i
 		a[i], a[opp] = a[opp], a[i]
 	}
 

diff --git a/state/pkg/resolve_dependencies_test.go b/state/pkg/resolve_dependencies_test.go
@@ -66,7 +66,7 @@ var _ = Describe("DependencyResolver", func() {
 		Expect(deps).To(Equal([]*birelpkg.Package{&c, &b}))
 	})
 
-	It("supports diamond cycles", func(){
+	It("supports diamond cycles", func() {
 		a := birelpkg.Package{Name: "a"}
 		b := birelpkg.Package{Name: "b"}
 		c := birelpkg.Package{Name: "c"}
@@ -82,7 +82,7 @@ var _ = Describe("DependencyResolver", func() {
 		Expect(deps).To(Equal([]*birelpkg.Package{&b, &d, &c}))
 	})
 
-	It("supports sibling dependencies", func(){
+	It("supports sibling dependencies", func() {
 		a := birelpkg.Package{Name: "a"}
 		b := birelpkg.Package{Name: "b"}
 		c := birelpkg.Package{Name: "c"}