Drop error from changeset.Get signature (#2549)

* Drop error from changeset.Get signature

We simply return 'unknown'

* update mako/sidecar's use of changeset

changeset/commit.go

@@ -17,7 +17,6 @@ limitations under the License.
 package changeset
 
 import (
-	"errors"
 	"regexp"
 	"runtime/debug"
 	"strconv"
@@ -32,26 +31,25 @@ var (
 	err       error
 	once      sync.Once
 
-	// for testing
 	readBuildInfo = debug.ReadBuildInfo
 )
 
 // Get returns the 'vcs.revision' property from the embedded build information
-// This function will return an error if value is not a valid Git SHA
+// If there is no embedded information 'unknown' will be returned
 //
 // The result will have a '-dirty' suffix if the workspace was not clean
-func Get() (string, error) {
+func Get() string {
 	once.Do(func() {
-		rev, err = get()
+		rev = get()
 	})
 
-	return rev, err
+	return rev
 }
 
-func get() (string, error) {
+func get() string {
 	info, ok := readBuildInfo()
 	if !ok {
-		return "", errors.New("unable to read build info")
+		return Unknown
 	}
 
 	var revision string
@@ -67,7 +65,7 @@ func get() (string, error) {
 	}
 
 	if revision == "" {
-		return Unknown, nil
+		return Unknown
 	}
 
 	if shaRegexp.MatchString(revision) {
@@ -78,5 +76,5 @@ func get() (string, error) {
 		revision += "-dirty"
 	}
 
-	return revision, nil
+	return revision
 }

changeset/commit_test.go

@@ -27,15 +27,14 @@ import (
 func TestGet(t *testing.T) {
 
 	cases := []struct {
-		name    string
-		info    *debug.BuildInfo
-		ok      bool
-		result  string
-		wantErr string
+		name   string
+		info   *debug.BuildInfo
+		ok     bool
+		result string
 	}{{
-		name:    "info fails",
-		ok:      false,
-		wantErr: "unable to read build info",
+		name:   "info fails",
+		ok:     false,
+		result: Unknown,
 	}, {
 		name:   "missing revision",
 		ok:     true,
@@ -79,18 +78,10 @@ func TestGet(t *testing.T) {
 				return c.info, c.ok
 			}
 
-			val, err := Get()
-			if c.wantErr == "" && err != nil {
-				t.Fatal("unexpected error", err)
-			} else if c.wantErr != "" && err != nil {
-				if diff := cmp.Diff(c.wantErr, err.Error()); diff != "" {
-					t.Fatalf("error doesn't match expected: %s", diff)
-				}
-			} else if c.wantErr != "" && err == nil {
-				t.Fatalf("expected error %q but was nil", c.wantErr)
-			}
+			got := Get()
+			want := c.result
 
-			if diff := cmp.Diff(c.result, val); diff != "" {
+			if diff := cmp.Diff(want, got); diff != "" {
 				t.Errorf("result doesn't match expected: %s", diff)
 			}
 		})

logging/config.go

@@ -75,15 +75,14 @@ func NewLogger(configJSON string, levelOverride string, opts ...zap.Option) (*za
 }
 
 func enrichLoggerWithCommitID(logger *zap.Logger) *zap.SugaredLogger {
-	revision, err := changeset.Get()
-	if err != nil || revision == changeset.Unknown {
-		logger.Info("Unable to read vcs.revision from binary", zap.Error(err))
+	revision := changeset.Get()
+	if revision == changeset.Unknown {
+		logger.Info("Unable to read vcs.revision from binary")
 		return logger.Sugar()
 	}
 
 	// Enrich logs with the components git revision.
 	return logger.With(zap.String(logkey.Commit, revision)).Sugar()
-
 }
 
 // NewLoggerFromConfig creates a logger using the provided Config

test/mako/sidecar.go

@@ -91,8 +91,8 @@ func EscapeTag(tag string) string {
 func SetupHelper(ctx context.Context, benchmarkKey *string, benchmarkName *string, extraTags ...string) (*Client, error) {
 	tags := append(config.MustGetTags(), extraTags...)
 	// Get the commit of the benchmarks
-	commitID, err := changeset.Get()
-	if err != nil {
+	commitID := changeset.Get()
+	if commitID == changeset.Unknown {
 		log.Println("Cannot find commit ID")
 	}