Merge pull request #21 from bvieira/#20

Bugfix: Can't override branches.skip with an empty array

cmd/git-sv/config.go

@@ -6,9 +6,11 @@ import (
 	"io/ioutil"
 	"log"
 	"os/exec"
+	"reflect"
 	"strings"
 	"sv4git/sv"
 
+	"github.com/imdario/mergo"
 	"github.com/kelseyhightower/envconfig"
 	"gopkg.in/yaml.v3"
 )
@@ -90,3 +92,37 @@ func defaultConfig() Config {
 		},
 	}
 }
+
+func merge(dst *Config, src Config) error {
+	err := mergo.Merge(dst, src, mergo.WithOverride, mergo.WithTransformers(&mergeTransformer{}))
+	if err == nil {
+		if len(src.ReleaseNotes.Headers) > 0 { // mergo is merging maps, ReleaseNotes.Headers should be overwritten
+			dst.ReleaseNotes.Headers = src.ReleaseNotes.Headers
+		}
+	}
+	return err
+}
+
+type mergeTransformer struct {
+}
+
+func (t *mergeTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error {
+	if typ.Kind() == reflect.Slice {
+		return func(dst, src reflect.Value) error {
+			if dst.CanSet() && !src.IsNil() {
+				dst.Set(src)
+			}
+			return nil
+		}
+	}
+
+	if typ.Kind() == reflect.Ptr {
+		return func(dst, src reflect.Value) error {
+			if dst.CanSet() && !src.IsNil() {
+				dst.Set(src)
+			}
+			return nil
+		}
+	}
+	return nil
+}

cmd/git-sv/config_test.go

@@ -0,0 +1,47 @@
+package main
+
+import (
+	"reflect"
+	"sv4git/sv"
+	"testing"
+)
+
+func Test_merge(t *testing.T) {
+	boolFalse := false
+	boolTrue := true
+
+	tests := []struct {
+		name    string
+		dst     Config
+		src     Config
+		want    Config
+		wantErr bool
+	}{
+		{"overwrite string", Config{Version: "a"}, Config{Version: "b"}, Config{Version: "b"}, false},
+		{"default string", Config{Version: "a"}, Config{Version: ""}, Config{Version: "a"}, false},
+
+		{"overwrite list", Config{Branches: sv.BranchesConfig{Skip: []string{"a", "b"}}}, Config{Branches: sv.BranchesConfig{Skip: []string{"c", "d"}}}, Config{Branches: sv.BranchesConfig{Skip: []string{"c", "d"}}}, false},
+		{"overwrite list with empty", Config{Branches: sv.BranchesConfig{Skip: []string{"a", "b"}}}, Config{Branches: sv.BranchesConfig{Skip: make([]string, 0)}}, Config{Branches: sv.BranchesConfig{Skip: make([]string, 0)}}, false},
+		{"default list", Config{Branches: sv.BranchesConfig{Skip: []string{"a", "b"}}}, Config{Branches: sv.BranchesConfig{Skip: nil}}, Config{Branches: sv.BranchesConfig{Skip: []string{"a", "b"}}}, false},
+
+		{"overwrite pointer bool false", Config{Branches: sv.BranchesConfig{SkipDetached: &boolFalse}}, Config{Branches: sv.BranchesConfig{SkipDetached: &boolTrue}}, Config{Branches: sv.BranchesConfig{SkipDetached: &boolTrue}}, false},
+		{"overwrite pointer bool true", Config{Branches: sv.BranchesConfig{SkipDetached: &boolTrue}}, Config{Branches: sv.BranchesConfig{SkipDetached: &boolFalse}}, Config{Branches: sv.BranchesConfig{SkipDetached: &boolFalse}}, false},
+		{"default pointer bool", Config{Branches: sv.BranchesConfig{SkipDetached: &boolTrue}}, Config{Branches: sv.BranchesConfig{SkipDetached: nil}}, Config{Branches: sv.BranchesConfig{SkipDetached: &boolTrue}}, false},
+
+		{"merge maps", Config{CommitMessage: sv.CommitMessageConfig{Footer: map[string]sv.CommitMessageFooterConfig{"issue": {Key: "jira"}}}}, Config{CommitMessage: sv.CommitMessageConfig{Footer: map[string]sv.CommitMessageFooterConfig{"issue2": {Key: "jira2"}}}}, Config{CommitMessage: sv.CommitMessageConfig{Footer: map[string]sv.CommitMessageFooterConfig{"issue": {Key: "jira"}, "issue2": {Key: "jira2"}}}}, false},
+		{"default maps", Config{CommitMessage: sv.CommitMessageConfig{Footer: map[string]sv.CommitMessageFooterConfig{"issue": {Key: "jira"}}}}, Config{CommitMessage: sv.CommitMessageConfig{Footer: nil}}, Config{CommitMessage: sv.CommitMessageConfig{Footer: map[string]sv.CommitMessageFooterConfig{"issue": {Key: "jira"}}}}, false},
+		{"merge empty maps", Config{CommitMessage: sv.CommitMessageConfig{Footer: map[string]sv.CommitMessageFooterConfig{"issue": {Key: "jira"}}}}, Config{CommitMessage: sv.CommitMessageConfig{Footer: map[string]sv.CommitMessageFooterConfig{}}}, Config{CommitMessage: sv.CommitMessageConfig{Footer: map[string]sv.CommitMessageFooterConfig{"issue": {Key: "jira"}}}}, false},
+
+		{"overwrite release notes header", Config{ReleaseNotes: sv.ReleaseNotesConfig{Headers: map[string]string{"a": "aa"}}}, Config{ReleaseNotes: sv.ReleaseNotesConfig{Headers: map[string]string{"b": "bb"}}}, Config{ReleaseNotes: sv.ReleaseNotesConfig{Headers: map[string]string{"b": "bb"}}}, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if err := merge(&tt.dst, tt.src); (err != nil) != tt.wantErr {
+				t.Errorf("merge() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if !reflect.DeepEqual(tt.dst, tt.want) {
+				t.Errorf("merge() = %v, want %v", tt.dst, tt.want)
+			}
+		})
+	}
+}

cmd/git-sv/main.go

@@ -4,10 +4,8 @@ import (
 	"log"
 	"os"
 	"path/filepath"
-	"reflect"
 	"sv4git/sv"
 
-	"github.com/imdario/mergo"
 	"github.com/urfave/cli/v2"
 )
 
@@ -28,7 +26,7 @@ func main() {
 
 	if envCfg.Home != "" {
 		if homeCfg, err := loadConfig(filepath.Join(envCfg.Home, configFilename)); err == nil {
-			if merr := mergo.Merge(&cfg, homeCfg, mergo.WithOverride, mergo.WithTransformers(&nullTransformer{})); merr != nil {
+			if merr := merge(&cfg, homeCfg); merr != nil {
 				log.Fatal(merr)
 			}
 		}
@@ -40,7 +38,7 @@ func main() {
 	}
 
 	if repoCfg, err := loadConfig(filepath.Join(repoPath, repoConfigFilename)); err == nil {
-		if merr := mergo.Merge(&cfg, repoCfg, mergo.WithOverride, mergo.WithTransformers(&nullTransformer{})); merr != nil {
+		if merr := merge(&cfg, repoCfg); merr != nil {
 			log.Fatal(merr)
 		}
 		if len(repoCfg.ReleaseNotes.Headers) > 0 { // mergo is merging maps, headers will be overwritten
@@ -161,18 +159,3 @@ func main() {
 		log.Fatal(apperr)
 	}
 }
-
-type nullTransformer struct {
-}
-
-func (t *nullTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error {
-	if typ.Kind() == reflect.Ptr {
-		return func(dst, src reflect.Value) error {
-			if dst.CanSet() && !src.IsNil() {
-				dst.Set(src)
-			}
-			return nil
-		}
-	}
-	return nil
-}