Merge branch 'beholders_eye/chore/errorf-go-best-practice' into nextyb

buildpacks/golang.go

@@ -111,7 +111,7 @@ func (bt GolangBuildTool) Install(ctx context.Context) (string, error) {
 	log.Infof("Downloading from URL %s ...", downloadURL)
 	localFile, err := t.DownloadFile(ctx, downloadURL)
 	if err != nil {
-		log.Errorf("Error downloading file: %v", err)
+		log.Errorf("Unable to download: %v", err)
 		return "", err
 	}
 

buildpacks/homebrew.go

@@ -71,19 +71,19 @@ func (bt HomebrewBuildTool) PackagePrefix(ctx context.Context, packageString str
 
 	err := t.Run(ctx, p)
 	if err != nil {
-		return "", fmt.Errorf("Couldn't get prefix for package %s: %v", bt.pkgName, err)
+		return "", fmt.Errorf("getting prefix for package %s: %v", bt.pkgName, err)
 	}
 
 	err = bufWriter.Flush()
 	if err != nil {
-		return "", fmt.Errorf("Couldn't get prefix for package %s: %v", bt.pkgName, err)
+		return "", fmt.Errorf("getting prefix for package %s: %v", bt.pkgName, err)
 	}
 
 	rd := bufio.NewReader(&buf)
 
 	prefixPath, err := rd.ReadString('\n')
 	if err != nil {
-		return "", fmt.Errorf("Couldn't get prefix for package %s: %v", bt.pkgName, err)
+		return "", fmt.Errorf("getting prefix for package %s: %v", bt.pkgName, err)
 	}
 	prefixPath = strings.TrimSuffix(prefixPath, "\n")
 	bt.pkgPrefix = prefixPath
@@ -134,11 +134,11 @@ func (bt HomebrewBuildTool) Install(ctx context.Context) (string, error) {
 	case "linux":
 		err = bt.installLinux(ctx, brewDir)
 	default:
-		err = fmt.Errorf("Unsupported platform: %s", gi.GoOS)
+		err = fmt.Errorf("unsupported platform: %s", gi.GoOS)
 	}
 
 	if err != nil {
-		return "", fmt.Errorf("Unable to install Homebrew: %v", err)
+		return "", fmt.Errorf("installing Homebrew: %v", err)
 	}
 
 	if bt.IsPackage() {
@@ -167,7 +167,7 @@ func (bt HomebrewBuildTool) InstallPackage(ctx context.Context, brewDir string)
 	}
 	err := t.Run(ctx, p)
 	if err != nil {
-		return fmt.Errorf("Couldn't update brew: %v", err)
+		return fmt.Errorf("updating brew: %v", err)
 	}
 
 	pkgVersion := ""
@@ -180,7 +180,7 @@ func (bt HomebrewBuildTool) InstallPackage(ctx context.Context, brewDir string)
 	err = t.Run(ctx, p)
 
 	if err != nil {
-		return fmt.Errorf("Couldn't intall %s@%s from  Homebrew: %v", bt.pkgName, bt.version, err)
+		return fmt.Errorf("installing %s@%s from Homebrew: %v", bt.pkgName, bt.version, err)
 	}
 
 	return nil
@@ -203,7 +203,7 @@ func (bt HomebrewBuildTool) installDarwin(ctx context.Context, brewDir string) e
 
 		if err != nil {
 			log.Errorf("Unable to clone brew!")
-			return fmt.Errorf("Couldn't clone brew: %v", err)
+			return fmt.Errorf("trying to clone brew: %v", err)
 		}
 	}
 
@@ -236,7 +236,7 @@ func (bt HomebrewBuildTool) installLinux(ctx context.Context, brewDir string) er
 
 		if err != nil {
 			log.Errorf("Unable to clone brew!")
-			return fmt.Errorf("Couldn't clone brew: %v", err)
+			return fmt.Errorf("trying to clone brew: %v", err)
 		}
 
 		log.Infof("Updating brew")
@@ -256,7 +256,7 @@ func (bt HomebrewBuildTool) Setup(ctx context.Context, brewDir string) error {
 	if bt.IsPackage() {
 		prefixPath, err := bt.PackagePrefix(ctx, bt.PackageVersionString())
 		if err != nil {
-			return fmt.Errorf("Unable to determine prefix for package %s: %v", bt.PackageVersionString(), err)
+			return fmt.Errorf("determining prefix for package %s: %v", bt.PackageVersionString(), err)
 		}
 		binDir := filepath.Join(prefixPath, "bin")
 		sbinDir := filepath.Join(prefixPath, "sbin")

buildpacks/python.go

@@ -69,7 +69,7 @@ func (bt PythonBuildTool) Install(ctx context.Context) (string, error) {
 			Directory: setupDir,
 		}
 		if err := t.Run(ctx, p); err != nil {
-			return "", fmt.Errorf("Couldn't install python: %v", err)
+			return "", fmt.Errorf("installing python: %v", err)
 		}
 	}
 
@@ -145,7 +145,7 @@ func (bt PythonBuildTool) Setup(ctx context.Context, condaDir string) error {
 
 			if err := t.Run(ctx, p); err != nil {
 				log.Errorf("Unable to run setup command: %s", cmd)
-				return fmt.Errorf("Unable to run '%s': %v", cmd, err)
+				return fmt.Errorf("running '%s': %v", cmd, err)
 			}
 		}
 	}

buildpacks/ruby.go

@@ -138,7 +138,7 @@ func (bt RubyBuildTool) Install(ctx context.Context) (string, error) {
 
 		if err != nil {
 			log.Infof("Unable to clone rbenv!")
-			return "", fmt.Errorf("Couldn't clone rbenv: %v", err)
+			return "", fmt.Errorf("trying to clone rbenv: %v", err)
 		}
 	}
 
@@ -160,7 +160,7 @@ func (bt RubyBuildTool) Install(ctx context.Context) (string, error) {
 
 		if err != nil {
 			log.Errorf("Unable to clone ruby-build!")
-			return "", fmt.Errorf("Couldn't clone ruby-build: %v", err)
+			return "", fmt.Errorf("trying to clone ruby-build: %v", err)
 		}
 	}
 

buildpacks/yarn.go

@@ -2,7 +2,6 @@ package buildpacks
 
 import (
 	"context"
-	"fmt"
 	"path/filepath"
 
 	"github.com/yourbase/yb/plumbing/log"
@@ -58,11 +57,13 @@ func (bt YarnBuildTool) Install(ctx context.Context) (string, error) {
 	log.Infof("Downloading from URL %s...", downloadURL)
 	localFile, err := t.DownloadFile(ctx, downloadURL)
 	if err != nil {
-		return "", fmt.Errorf("Unable to download %s: %v", downloadURL, err)
+		log.Errorf("Unable to download: %v", err)
+		return "", err
 	}
 
 	if err := t.Unarchive(ctx, localFile, installDir); err != nil {
-		return "", fmt.Errorf("Unable to decompress archive: %v", err)
+		log.Errorf("Unable to decompress: %v", err)
+		return "", err
 	}
 
 	return yarnDir, nil

cli/build.go

@@ -184,7 +184,7 @@ func parseArgs(lonelyArg string) (pkgName, target string, err error) {
 
 		parts := strings.SplitN(strings.TrimPrefix(lonelyArg, "@"), ":", 2)
 		if len(parts) < 2 {
-			err = fmt.Errorf("unable to parse package/target definition: %s", lonelyArg)
+			err = fmt.Errorf("parsing package/target definition: %s", lonelyArg)
 			return
 		}
 		pkgName = parts[0]

cli/helpers.go

@@ -23,7 +23,7 @@ func GetTargetPackageNamed(file string) (Package, error) {
 		_, pkgName := filepath.Split(currentPath)
 		pkg, err := LoadPackage(pkgName, currentPath)
 		if err != nil {
-			return Package{}, fmt.Errorf("Error loading package '%s': %v\n\nSee %s\n", pkgName, err, DOCS_URL)
+			return Package{}, fmt.Errorf("loading package '%s': %v\n\nSee %s\n", pkgName, err, DOCS_URL)
 		}
 		targetPackage = pkg
 	} else {
@@ -32,12 +32,12 @@ func GetTargetPackageNamed(file string) (Package, error) {
 
 		if err != nil {
 
-			return Package{}, fmt.Errorf("Could not find valid configuration: %v\n\nTry running in the package root dir or writing the YML config file (%s) if it is missing. See %s", err, file, DOCS_URL)
+			return Package{}, fmt.Errorf("finding valid configuration: %v\n\nTry running in the package root dir or writing the YML config file (%s) if it is missing. See %s", err, file, DOCS_URL)
 		}
 
 		pkg, err := workspace.TargetPackage()
 		if err != nil {
-			return Package{}, fmt.Errorf("Can't load workspace's target package: %v\n\nPackages under this Workspace may be missing a %s file or it's syntax is an invalid YML data. See %s", err, file, DOCS_URL)
+			return Package{}, fmt.Errorf("loading workspace's target package: %v\n\nPackages under this Workspace may be missing a %s file or it's syntax is an invalid YML data. See %s", err, file, DOCS_URL)
 		}
 
 		targetPackage = pkg

config/config_file.go

@@ -30,7 +30,7 @@ func configFilePath() (string, error) {
 func loadConfigFile() (*ini.File, error) {
 	iniPath, err := configFilePath()
 	if err != nil {
-		return nil, fmt.Errorf("Couldn't determine config file path: %v", err)
+		return nil, fmt.Errorf("determining config file path: %v", err)
 	}
 
 	cfg, err := ini.Load(iniPath)
@@ -57,7 +57,7 @@ func pathExists(path string) bool {
 func mkdirAsNeeded(dir string) error {
 	if _, err := os.Stat(dir); os.IsNotExist(err) {
 		if err := os.MkdirAll(dir, 0700); err != nil {
-			return fmt.Errorf("Unable to create dir: %v", err)
+			return fmt.Errorf("creating dir: %v", err)
 		}
 	}
 

config/settings.go

@@ -80,10 +80,10 @@ func apiBaseUrl() (string, error) {
 	case "":
 		return "https://api.yourbase.io", nil
 	default:
-		return "", fmt.Errorf("Unknown environment (%s) and no override in the config file or environment available", profile)
+		return "", fmt.Errorf("unknown environment (%s) and no override in the config file or environment available", profile)
 	}
 
-	return "", fmt.Errorf("Unable to generate URL")
+	return "", fmt.Errorf("generating URL")
 }
 
 func ApiUrl(path string) (string, error) {
@@ -92,7 +92,7 @@ func ApiUrl(path string) (string, error) {
 	}
 
 	if baseUrl, err := apiBaseUrl(); err != nil {
-		return "", fmt.Errorf("Can't determine API URL: %v", err)
+		return "", fmt.Errorf("determining API URL: %v", err)
 	} else {
 		return fmt.Sprintf("%s%s", baseUrl, path), nil
 	}
@@ -127,10 +127,10 @@ func managementBaseUrl() (string, error) {
 	case "":
 		return "https://app.yourbase.io", nil
 	default:
-		return "", fmt.Errorf("Unknown environment (%s) and no override in the config file or environment available", profile)
+		return "", fmt.Errorf("unknown environment (%s) and no override in the config file or environment available", profile)
 	}
 
-	return "", fmt.Errorf("Unable to generate URL")
+	return "", fmt.Errorf("generating URL")
 }
 
 func ManagementUrl(path string) (string, error) {
@@ -139,7 +139,7 @@ func ManagementUrl(path string) (string, error) {
 	}
 
 	if baseUrl, err := managementBaseUrl(); err != nil {
-		return "", fmt.Errorf("Couldn't determine management URL: %v", err)
+		return "", fmt.Errorf("determining management URL: %v", err)
 	} else {
 		return fmt.Sprintf("%s%s", baseUrl, path), nil
 	}
@@ -174,13 +174,13 @@ func UserToken() (string, error) {
 		token, err := GetConfigValue("user", "api_key")
 
 		if err != nil {
-			return "", fmt.Errorf("Unable to find YB token in config file or environment.\nUse yb login to fetch one, if you already logged in to https://app.yourbase.io")
+			return "", fmt.Errorf("finding YB token in config file or environment.\nUse yb login to fetch one, if you already logged in to https://app.yourbase.io")
 		}
 
 		return token, nil
 	} else {
 		return token, nil
 	}
 
-	return "", fmt.Errorf("Unable to determine token - not in the config or environment")
+	return "", fmt.Errorf("determining token - not in the config or environment")
 }

plumbing/patch_handle.go

@@ -15,17 +15,17 @@ import (
 
 func applyPatch(file *diffparser.DiffFile, from string) (to string, err error) {
 	if file == nil {
-		return "", fmt.Errorf("Needs a file to process")
+		return "", fmt.Errorf("no file to process")
 	}
 	lines := strings.Split(from, "\n")
 	if len(lines) == 0 {
-		return "", fmt.Errorf("Won't process an empty string")
+		return "", fmt.Errorf("empty string")
 	}
 	var unmatchedLines int64
 	for _, hunk := range file.Hunks {
 		idx := hunk.OrigRange.Start - 1
 		if idx < 0 {
-			return "", fmt.Errorf("Malformed patch, wrong start position (%d): %v\n", idx, strings.Join(file.Changes, "\n"))
+			return "", fmt.Errorf("wrong start position (%d): %v\n", idx, strings.Join(file.Changes, "\n"))
 		}
 		for i, line := range hunk.OrigRange.Lines {
 			index := idx + i
@@ -61,13 +61,13 @@ func applyPatch(file *diffparser.DiffFile, from string) (to string, err error) {
 //   If the worktree isn't passsed it will try working on the local directory
 func UnifiedPatchOnGit(patch string, commit *object.Commit, w, originWorktree *git.Worktree, wd string) (patchError error) {
 	if commit == nil && w == nil && wd == "" {
-		return fmt.Errorf("Nowhere to apply the patch on, please pass a git commit + git worktree, or a workdir to work on files")
+		return fmt.Errorf("no git commit + git worktree, or a workdir")
 	}
 
 	getCommitFileContents := func(file *diffparser.DiffFile) (contents string) {
 		tree, err := commit.Tree()
 		if err != nil {
-			patchError = fmt.Errorf("Unable to resolve commit tree: %v", err)
+			patchError = fmt.Errorf("resolving commit tree: %v", err)
 			return ""
 		}
 		var workFile *object.File
@@ -77,14 +77,14 @@ func UnifiedPatchOnGit(patch string, commit *object.Commit, w, originWorktree *g
 		case diffparser.MODIFIED:
 			workFile, err = tree.File(file.OrigName)
 			if err != nil {
-				patchError = fmt.Errorf("Unable to retrieve tree entry %s: %v", file.OrigName, err)
+				patchError = fmt.Errorf("retrieving tree entry %s: %v", file.OrigName, err)
 				return ""
 			}
 			contents, err = workFile.Contents()
 		case diffparser.NEW:
 			newFile, err := originWorktree.Filesystem.Open(file.NewName)
 			if err != nil {
-				patchError = fmt.Errorf("Unable to open %s on the work tree: %v", file.NewName, err)
+				patchError = fmt.Errorf("opening %s in the work tree: %v", file.NewName, err)
 				return ""
 			}
 			newBytes := bytes.NewBuffer(nil)
@@ -93,7 +93,7 @@ func UnifiedPatchOnGit(patch string, commit *object.Commit, w, originWorktree *g
 			_ = newFile.Close()
 		}
 		if err != nil {
-			patchError = fmt.Errorf("Couldn't get contents of %s: %v", file.OrigName, err)
+			patchError = fmt.Errorf("geting contents of %s: %v", file.OrigName, err)
 			return ""
 		}
 		return
@@ -102,7 +102,7 @@ func UnifiedPatchOnGit(patch string, commit *object.Commit, w, originWorktree *g
 	// Detect in the patch string (unified) which files were affected and how
 	diff, err := diffparser.Parse(patch)
 	if err != nil {
-		return fmt.Errorf("Generated patch parse failed: %v", err)
+		return fmt.Errorf("patch parsing: %v", err)
 	}
 
 	for _, file := range diff.Files {
@@ -111,25 +111,25 @@ func UnifiedPatchOnGit(patch string, commit *object.Commit, w, originWorktree *g
 		case diffparser.DELETED:
 			w.Remove(file.OrigName)
 		case diffparser.MOVED:
-			return fmt.Errorf("Not implemented yet")
+			return fmt.Errorf("not implemented yet")
 		default:
 			contents := getCommitFileContents(file)
 			if contents == "" && patchError != nil {
-				return fmt.Errorf("Unable to fetch contents from %s: %v", file, patchError)
+				return fmt.Errorf("fetching contents from %s: %v", file, patchError)
 			}
 
 			var fixedContents string
 			if file.Mode == diffparser.MODIFIED {
 				fixedContents, err = applyPatch(file, contents)
 				if err != nil {
-					return fmt.Errorf("Apply Patch failed for <%s>: %v", file, err)
+					return fmt.Errorf("applying patch for <%s>: %v", file, err)
 				}
 			}
 
 			if w != nil {
 				newFile, err := w.Filesystem.Create(file.NewName)
 				if err != nil {
-					return fmt.Errorf("Unable to open %s on the cloned tree: %v", file.NewName, err)
+					return fmt.Errorf("opening %s: %v", file.NewName, err)
 				}
 
 				var c string
@@ -139,14 +139,14 @@ func UnifiedPatchOnGit(patch string, commit *object.Commit, w, originWorktree *g
 					c = contents
 				}
 				if _, err = newFile.Write([]byte(c)); err != nil {
-					return fmt.Errorf("Unable to write patch hunk to %s: %v", file.NewName, err)
+					return fmt.Errorf("writing patch hunk to %s: %v", file.NewName, err)
 				}
 				_ = newFile.Close()
 
 				w.Add(file.NewName)
 			} else if wd != "" {
 				//TODO Change file contents directly on the directory
-				return fmt.Errorf("Not implemented")
+				return fmt.Errorf("not implemented")
 			}
 		}
 	}

plumbing/progress.go

@@ -1,5 +1,5 @@
 // This came from https://raw.githubusercontent.com/BTBurke/clt/7f5151b6bef1b0da4b03f965b47cbe2de87a3ac0/progress.go
-// NOTE the next steps in the field of UX for the CLI, we should consider moving to use clt
+// TODO use github.com/urfave/cli/v2 + github.com/manifoldco/promptui + github.com/vbauerster/mpb
 package plumbing
 
 import (