Skip to content

Commit 0dbaf7e

Browse files
committed
Refactor code using gofmt and go lint.
This Commit, - Refactors code using `gofmt` and `go lint` to improve code quality. - Relocates `diff` and `manifests` into `pkg/` dir. - Adds scripts to `gofmt` code and to update required changes - Adds scripts to `go lint` and `go vet` code - Adds `lint` stage before `build` to retain code quality
1 parent 28e6832 commit 0dbaf7e

22 files changed

+124
-45
lines changed

Makefile

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,15 @@ install: build
2020
cp bin/diff $(HELM_HOME)/plugins/helm-diff/bin
2121
cp plugin.yaml $(HELM_HOME)/plugins/helm-diff/
2222

23+
.PHONY: lint
24+
lint:
25+
scripts/update-gofmt.sh
26+
scripts/verify-gofmt.sh
27+
scripts/verify-golint.sh
28+
scripts/verify-govet.sh
29+
2330
.PHONY: build
24-
build:
31+
build: lint
2532
mkdir -p bin/
2633
go build -i -v -o bin/diff -ldflags="$(LDFLAGS)"
2734

@@ -61,7 +68,7 @@ dist:
6168
tar -C build/ -zcvf $(CURDIR)/release/helm-diff-windows.tgz diff/
6269

6370
.PHONY: release
64-
release: dist
71+
release: lint dist
6572
ifndef GITHUB_TOKEN
6673
$(error GITHUB_TOKEN is undefined)
6774
endif

cmd/error.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package cmd
22

3+
// Error to report errors
34
type Error struct {
45
error
56
Code int

cmd/helm.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ func (v *valueFiles) Valid() error {
4141

4242
if errStr == "" {
4343
return nil
44-
} else {
45-
return errors.New(errStr)
4644
}
45+
46+
return errors.New(errStr)
4747
}
4848

4949
func (v *valueFiles) Type() string {
@@ -148,12 +148,12 @@ func (d *diffCmd) vals() ([]byte, error) {
148148
currentMap := map[string]interface{}{}
149149

150150
var bytes []byte
151-
var err error
152-
if strings.TrimSpace(filePath) == "-" {
153-
bytes, err = ioutil.ReadAll(os.Stdin)
154-
} else {
155-
bytes, err = ioutil.ReadFile(filePath)
156-
}
151+
var err error
152+
if strings.TrimSpace(filePath) == "-" {
153+
bytes, err = ioutil.ReadAll(os.Stdin)
154+
} else {
155+
bytes, err = ioutil.ReadFile(filePath)
156+
}
157157
if err != nil {
158158
return []byte{}, err
159159
}

cmd/helpers.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ const (
1919
)
2020

2121
var (
22-
settings helm_env.EnvSettings
22+
settings helm_env.EnvSettings
23+
// DefaultHelmHome to hold default home path of .helm dir
2324
DefaultHelmHome = filepath.Join(homedir.HomeDir(), ".helm")
2425
)
2526

@@ -59,7 +60,7 @@ func createHelmClient() helm.Interface {
5960
}
6061

6162
func expandTLSPaths() {
62-
settings.TLSCaCertFile = os.ExpandEnv(settings.TLSCaCertFile)
63-
settings.TLSCertFile = os.ExpandEnv(settings.TLSCertFile)
64-
settings.TLSKeyFile = os.ExpandEnv(settings.TLSKeyFile)
63+
settings.TLSCaCertFile = os.ExpandEnv(settings.TLSCaCertFile)
64+
settings.TLSCertFile = os.ExpandEnv(settings.TLSCertFile)
65+
settings.TLSKeyFile = os.ExpandEnv(settings.TLSKeyFile)
6566
}

cmd/release.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import (
88
"github.com/spf13/cobra"
99
"k8s.io/helm/pkg/helm"
1010

11-
"github.com/databus23/helm-diff/diff"
12-
"github.com/databus23/helm-diff/manifest"
11+
"github.com/databus23/helm-diff/pkg/diff"
12+
"github.com/databus23/helm-diff/pkg/manifest"
1313
)
1414

1515
type release struct {
@@ -91,7 +91,7 @@ func (d *release) differentiate() error {
9191
}
9292

9393
if releaseResponse1.Release.Chart.Metadata.Name == releaseResponse2.Release.Chart.Metadata.Name {
94-
seenAnyChanges := diff.DiffReleases(
94+
seenAnyChanges := diff.DifferenceReleases(
9595
manifest.ParseRelease(releaseResponse1.Release, d.includeTests),
9696
manifest.ParseRelease(releaseResponse2.Release, d.includeTests),
9797
d.suppressedKinds,

cmd/revision.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import (
99
"github.com/spf13/cobra"
1010
"k8s.io/helm/pkg/helm"
1111

12-
"github.com/databus23/helm-diff/diff"
13-
"github.com/databus23/helm-diff/manifest"
12+
"github.com/databus23/helm-diff/pkg/diff"
13+
"github.com/databus23/helm-diff/pkg/manifest"
1414
)
1515

1616
type revision struct {
@@ -104,7 +104,7 @@ func (d *revision) differentiate() error {
104104
return prettyError(err)
105105
}
106106

107-
diff.DiffManifests(
107+
diff.DifferenceManifests(
108108
manifest.ParseRelease(revisionResponse.Release, d.includeTests),
109109
manifest.ParseRelease(releaseResponse.Release, d.includeTests),
110110
d.suppressedKinds,
@@ -128,7 +128,7 @@ func (d *revision) differentiate() error {
128128
return prettyError(err)
129129
}
130130

131-
seenAnyChanges := diff.DiffManifests(
131+
seenAnyChanges := diff.DifferenceManifests(
132132
manifest.ParseRelease(revisionResponse1.Release, d.includeTests),
133133
manifest.ParseRelease(revisionResponse2.Release, d.includeTests),
134134
d.suppressedKinds,

cmd/rollback.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import (
99
"github.com/spf13/cobra"
1010
"k8s.io/helm/pkg/helm"
1111

12-
"github.com/databus23/helm-diff/diff"
13-
"github.com/databus23/helm-diff/manifest"
12+
"github.com/databus23/helm-diff/pkg/diff"
13+
"github.com/databus23/helm-diff/pkg/manifest"
1414
)
1515

1616
type rollback struct {
@@ -96,7 +96,7 @@ func (d *rollback) backcast() error {
9696
}
9797

9898
// create a diff between the current manifest and the version of the manifest that a user is intended to rollback
99-
seenAnyChanges := diff.DiffManifests(
99+
seenAnyChanges := diff.DifferenceManifests(
100100
manifest.ParseRelease(releaseResponse.Release, d.includeTests),
101101
manifest.ParseRelease(revisionResponse.Release, d.includeTests),
102102
d.suppressedKinds,

cmd/root.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ The Helm Diff Plugin
2525
helm rollback will perform.
2626
`
2727

28+
// New creates a new cobra client
2829
func New() *cobra.Command {
2930

3031
chartCommand := newChartCommand()

cmd/upgrade.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import (
99
"github.com/spf13/cobra"
1010
"k8s.io/helm/pkg/helm"
1111

12-
"github.com/databus23/helm-diff/diff"
13-
"github.com/databus23/helm-diff/manifest"
12+
"github.com/databus23/helm-diff/pkg/diff"
13+
"github.com/databus23/helm-diff/pkg/manifest"
1414
)
1515

1616
type diffCmd struct {
@@ -171,7 +171,7 @@ func (d *diffCmd) run() error {
171171
}
172172
}
173173

174-
seenAnyChanges := diff.DiffManifests(currentSpecs, newSpecs, d.suppressedKinds, d.outputContext, os.Stdout)
174+
seenAnyChanges := diff.DifferenceManifests(currentSpecs, newSpecs, d.suppressedKinds, d.outputContext, os.Stdout)
175175

176176
if d.detailedExitCode && seenAnyChanges {
177177
return Error{

diff/diff.go renamed to pkg/diff/diff.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,11 @@ import (
1010
"github.com/aryann/difflib"
1111
"github.com/mgutz/ansi"
1212

13-
"github.com/databus23/helm-diff/manifest"
13+
"github.com/databus23/helm-diff/pkg/manifest"
1414
)
1515

16-
func DiffManifests(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, context int, to io.Writer) bool {
16+
// DifferenceManifests diff on manifests
17+
func DifferenceManifests(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, context int, to io.Writer) bool {
1718
seenAnyChanges := false
1819
emptyMapping := &manifest.MappingResult{}
1920
for key, oldContent := range oldIndex {
@@ -52,11 +53,11 @@ func DiffManifests(oldIndex, newIndex map[string]*manifest.MappingResult, suppre
5253
return seenAnyChanges
5354
}
5455

55-
// DiffReleases reindex the content based on the template names and pass it to DiffManifests
56-
func DiffReleases(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, context int, to io.Writer) bool {
56+
// DifferenceReleases reindex the content based on the template names and pass it to DifferenceManifests
57+
func DifferenceReleases(oldIndex, newIndex map[string]*manifest.MappingResult, suppressedKinds []string, context int, to io.Writer) bool {
5758
oldIndex = reIndexForRelease(oldIndex)
5859
newIndex = reIndexForRelease(newIndex)
59-
return DiffManifests(oldIndex, newIndex, suppressedKinds, context, to)
60+
return DifferenceManifests(oldIndex, newIndex, suppressedKinds, context, to)
6061
}
6162

6263
func diffMappingResults(oldContent *manifest.MappingResult, newContent *manifest.MappingResult) []difflib.DiffRecord {

diff/diff_test.go renamed to pkg/diff/diff_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
"github.com/mgutz/ansi"
88
"github.com/stretchr/testify/require"
99

10-
"github.com/databus23/helm-diff/manifest"
10+
"github.com/databus23/helm-diff/pkg/manifest"
1111
)
1212

1313
var text1 = "" +
@@ -127,9 +127,9 @@ func assertDiff(t *testing.T, before, after string, context int, expected string
127127
}
128128
}
129129

130-
func TestDiffManifests(t *testing.T) {
130+
func TestDifferenceManifests(t *testing.T) {
131131
specBeta := map[string]*manifest.MappingResult{
132-
"default, nginx, Deployment (apps)": &manifest.MappingResult{
132+
"default, nginx, Deployment (apps)": {
133133

134134
Name: "default, nginx, Deployment (apps)",
135135
Kind: "Deployment",
@@ -142,7 +142,7 @@ metadata:
142142
}}
143143

144144
specRelease := map[string]*manifest.MappingResult{
145-
"default, nginx, Deployment (apps)": &manifest.MappingResult{
145+
"default, nginx, Deployment (apps)": {
146146

147147
Name: "default, nginx, Deployment (apps)",
148148
Kind: "Deployment",
@@ -158,8 +158,8 @@ metadata:
158158

159159
var buf1 bytes.Buffer
160160

161-
if changesSeen := DiffManifests(specBeta, specRelease, []string{}, 10, &buf1); !changesSeen {
162-
t.Error("Unexpected return value from DiffManifests: Expected the return value to be `true` to indicate that it has seen any change(s), but was `false`")
161+
if changesSeen := DifferenceManifests(specBeta, specRelease, []string{}, 10, &buf1); !changesSeen {
162+
t.Error("Unexpected return value from DifferenceManifests: Expected the return value to be `true` to indicate that it has seen any change(s), but was `false`")
163163
}
164164

165165
require.Equal(t, `default, nginx, Deployment (apps) has changed:
@@ -176,8 +176,8 @@ metadata:
176176
t.Run("OnNoChange", func(t *testing.T) {
177177
var buf2 bytes.Buffer
178178

179-
if changesSeen := DiffManifests(specRelease, specRelease, []string{}, 10, &buf2); changesSeen {
180-
t.Error("Unexpected return value from DiffManifests: Expected the return value to be `false` to indicate that it has NOT seen any change(s), but was `true`")
179+
if changesSeen := DifferenceManifests(specRelease, specRelease, []string{}, 10, &buf2); changesSeen {
180+
t.Error("Unexpected return value from DifferenceManifests: Expected the return value to be `false` to indicate that it has NOT seen any change(s), but was `true`")
181181
}
182182

183183
require.Equal(t, ``, buf2.String())

manifest/parse.go renamed to pkg/manifest/parse.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,15 @@ import (
1313

1414
var yamlSeperator = []byte("\n---\n")
1515

16+
// MappingResult to store result of diff
1617
type MappingResult struct {
1718
Name string
1819
Kind string
1920
Content string
2021
}
2122

2223
type metadata struct {
23-
ApiVersion string `yaml:"apiVersion"`
24+
APIVersion string `yaml:"apiVersion"`
2425
Kind string
2526
Metadata struct {
2627
Namespace string
@@ -29,7 +30,7 @@ type metadata struct {
2930
}
3031

3132
func (m metadata) String() string {
32-
apiBase := m.ApiVersion
33+
apiBase := m.APIVersion
3334
sp := strings.Split(apiBase, "/")
3435
if len(sp) > 1 {
3536
apiBase = strings.Join(sp[:len(sp)-1], "/")
@@ -61,6 +62,7 @@ func splitSpec(token string) (string, string) {
6162
return "", ""
6263
}
6364

65+
// ParseRelease parses release objects into MappingResult
6466
func ParseRelease(release *release.Release, includeTests bool) map[string]*MappingResult {
6567
manifest := release.Manifest
6668
for _, hook := range release.Hooks {
@@ -75,12 +77,13 @@ func ParseRelease(release *release.Release, includeTests bool) map[string]*Mappi
7577
return Parse(manifest, release.Namespace)
7678
}
7779

80+
// Parse parses manifest strings into MappingResult
7881
func Parse(manifest string, defaultNamespace string) map[string]*MappingResult {
7982
scanner := bufio.NewScanner(strings.NewReader(manifest))
8083
scanner.Split(scanYamlSpecs)
8184
//Allow for tokens (specs) up to 1M in size
8285
scanner.Buffer(make([]byte, bufio.MaxScanTokenSize), 1048576)
83-
//Discard the first result, we only care about everything after the first seperator
86+
//Discard the first result, we only care about everything after the first separator
8487
scanner.Scan()
8588

8689
result := make(map[string]*MappingResult)

manifest/parse_test.go renamed to pkg/manifest/parse_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ import (
77

88
"github.com/stretchr/testify/require"
99

10-
. "github.com/databus23/helm-diff/manifest"
10+
. "github.com/databus23/helm-diff/pkg/manifest"
1111
)
1212

1313
func foundObjects(result map[string]*MappingResult) []string {
1414
objs := make([]string, 0, len(result))
15-
for k, _ := range result {
15+
for k := range result {
1616
objs = append(objs, k)
1717
}
1818
sort.Strings(objs)
File renamed without changes.
File renamed without changes.

scripts/update-gofmt.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#!/bin/bash
2+
# script credits : https://github.com/infracloudio/botkube
3+
4+
set -o errexit
5+
set -o nounset
6+
set -o pipefail
7+
8+
find_files() {
9+
find . -not \( \
10+
\( \
11+
-wholename '*/vendor/*' \
12+
\) -prune \
13+
\) -name '*.go'
14+
}
15+
16+
find_files | xargs gofmt -w -s

scripts/verify-gofmt.sh

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#!/bin/bash
2+
# script credits : https://github.com/infracloudio/botkube
3+
4+
set -o errexit
5+
set -o nounset
6+
set -o pipefail
7+
8+
find_files() {
9+
find . -not \( \
10+
\( \
11+
-wholename '*/vendor/*' \
12+
\) -prune \
13+
\) -name '*.go'
14+
}
15+
16+
bad_files=$(find_files | xargs gofmt -d -s 2>&1)
17+
if [[ -n "${bad_files}" ]]; then
18+
echo "${bad_files}" >&2
19+
echo >&2
20+
echo "Run ./hack/update-gofmt.sh" >&2
21+
exit 1
22+
fi

scripts/verify-golint.sh

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#!/bin/bash
2+
# script credits : https://github.com/infracloudio/botkube
3+
4+
set -o errexit
5+
set -o nounset
6+
set -o pipefail
7+
8+
find_files() {
9+
find . -not \( \
10+
\( \
11+
-wholename '*/vendor/*' \
12+
\) -prune \
13+
\) -name '*.go'
14+
}
15+
16+
bad_files=$(find_files | xargs -I@ bash -c "golint @")
17+
if [[ -n "${bad_files}" ]]; then
18+
echo "${bad_files}"
19+
exit 1
20+
fi

scripts/verify-govet.sh

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#!/bin/bash
2+
# script credits : https://github.com/infracloudio/botkube
3+
4+
set -x
5+
6+
go vet ./pkg/...
7+
go vet ./cmd/...

0 commit comments

Comments
 (0)