Skip to content

Commit 023dd3e

Browse files
MadhavJivrajanidims
authored andcommitted
Address warnings and minor cleanups
* Majority of the changes include: - using strings.EqualFold for case insensitive matching - Removing nil checks for slices - strings.Contains instead of strings.Index to check for substring availability Signed-off-by: Madhav Jivrajani <[email protected]>
1 parent 78c7c69 commit 023dd3e

File tree

8 files changed

+42
-26
lines changed

8 files changed

+42
-26
lines changed

cmd/audit.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package cmd
1818

1919
import (
2020
"fmt"
21-
"github.com/pkg/errors"
2221
"io/ioutil"
2322
"net/http"
2423
"os"
@@ -30,6 +29,8 @@ import (
3029
"sync"
3130
"time"
3231

32+
"github.com/pkg/errors"
33+
3334
"github.com/spf13/cobra"
3435
"k8s.io/apimachinery/pkg/util/sets"
3536

@@ -96,13 +97,11 @@ func auditLocalOwnersFiles(context *utils.Context, args []string) {
9697
for _, groups := range context.PrefixToGroupMap() {
9798
for _, group := range groups {
9899
listOfGroups = append(listOfGroups, group.Dir)
99-
var files []string
100100
for _, subproject := range group.Subprojects {
101101
for _, owner := range subproject.Owners {
102102
if strings.Contains(owner, "/kubernetes/kubernetes/") {
103103
split := strings.SplitN(owner, "/", 7)
104104
filename := split[len(split)-1]
105-
files = append(files, filename)
106105
if val, ok := mapFilesToGroups[filename]; ok {
107106
val.Insert(group.Dir)
108107
} else {
@@ -172,7 +171,7 @@ func auditLocalOwnersFiles(context *utils.Context, args []string) {
172171
}
173172
}
174173
for _, line := range infoLog.List() {
175-
fmt.Printf(line)
174+
fmt.Println(line)
176175
}
177176
}
178177

cmd/export.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@ package cmd
1818

1919
import (
2020
"fmt"
21-
"github.com/spf13/cobra"
2221
"os"
2322
"sort"
2423
"strings"
2524
"time"
2625

26+
"github.com/spf13/cobra"
27+
2728
"k8s.io/apimachinery/pkg/util/sets"
2829

2930
"github.com/dims/maintainers/pkg/utils"
@@ -91,7 +92,7 @@ func exportOwnersAndAliases(pwd string) error {
9192
}
9293
userIDs.Insert(configOwners.Approvers...)
9394
userIDs.Insert(configOwners.Reviewers...)
94-
for key, _ := range repoAliases {
95+
for key := range repoAliases {
9596
if userIDs.Has(key) {
9697
userIDs.Delete(key)
9798
aliases.Insert(key)

cmd/prune.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@ package cmd
1818

1919
import (
2020
"fmt"
21-
"github.com/spf13/cobra"
2221
"os"
2322
"path/filepath"
2423
"sort"
2524
"strings"
2625
"time"
2726

27+
"github.com/spf13/cobra"
28+
2829
"k8s.io/apimachinery/pkg/util/sets"
2930

3031
"github.com/dims/maintainers/pkg/utils"
@@ -75,25 +76,39 @@ var pruneCmd = &cobra.Command{
7576
var ownerContribs []utils.Contribution
7677

7778
if !skipDS {
78-
err, contribs := utils.GetContributionsForAYear(repositoryDS, periodDS)
79+
contribs, err := utils.GetContributionsForAYear(repositoryDS, periodDS)
7980
if err != nil {
8081
panic(err)
8182
}
82-
if contribs == nil || len(contribs) == 0 {
83+
if len(contribs) == 0 {
8384
panic("unable to find any contributions in repository : " + repositoryDS)
8485
}
8586
for _, id := range uniqueUsers {
8687
for _, item := range contribs {
87-
if strings.ToLower(item.ID) == strings.ToLower(id) {
88-
ownerContribs = append(ownerContribs, utils.Contribution{id, item.ID, item.ContribCount, -1})
88+
if strings.EqualFold(item.ID, id) {
89+
ownerContribs = append(ownerContribs,
90+
utils.Contribution{
91+
ID: id,
92+
Alias: item.ID,
93+
ContribCount: item.ContribCount,
94+
CommentCount: -1,
95+
},
96+
)
8997
userIDs.Delete(id)
9098
break
9199
}
92100
}
93101
}
94102
} else {
95103
for _, id := range uniqueUsers {
96-
ownerContribs = append(ownerContribs, utils.Contribution{id, id, -1, -1})
104+
ownerContribs = append(ownerContribs,
105+
utils.Contribution{
106+
ID: id,
107+
Alias: id,
108+
ContribCount: -1,
109+
CommentCount: -1,
110+
},
111+
)
97112
userIDs.Delete(id)
98113
}
99114
}
@@ -233,7 +248,7 @@ func getOwnersAndAliases(pwd string) (sets.String, map[string][]string, []string
233248
userIDs.Insert(configOwners.Reviewers...)
234249
}
235250

236-
for key, _ := range repoAliases {
251+
for key := range repoAliases {
237252
userIDs.Delete(key)
238253
}
239254
if len(aliasPath) > 0 {

cmd/validate.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ var validateCmd = &cobra.Command{
7373
errors2 := warnFileMismatchesBetweenKubernetesRepoAndSigsYaml(fileMap)
7474
errors = append(errors, errors2...)
7575

76-
if errors != nil && len(errors) > 0 {
76+
if len(errors) > 0 {
7777
for _, err := range errors {
7878
fmt.Printf("WARNING: %v\n", err)
7979
}
@@ -90,7 +90,7 @@ func warnFileMismatchesBetweenKubernetesRepoAndSigsYaml(fileMap map[string]strin
9090
}
9191

9292
for key, val := range fileMap {
93-
if strings.Index(key, "kubernetes/kubernetes") == -1 {
93+
if !strings.Contains(key, "kubernetes/kubernetes") {
9494
continue
9595
}
9696
found := false

pkg/utils/devstats_utils.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,14 @@ import (
2020
"bytes"
2121
"encoding/json"
2222
"fmt"
23-
"github.com/pkg/errors"
2423
"io/ioutil"
2524
"net/http"
2625
"strings"
26+
27+
"github.com/pkg/errors"
2728
)
2829

29-
func GetContributionsForAYear(repository string, period string) (error, []Contribution) {
30+
func GetContributionsForAYear(repository string, period string) ([]Contribution, error) {
3031
postBody := `{
3132
"queries": [{
3233
"refId": "A",
@@ -47,22 +48,22 @@ func GetContributionsForAYear(repository string, period string) (error, []Contri
4748
requestBody := bytes.NewBuffer([]byte(postBody))
4849
resp, err := http.Post("https://k8s.devstats.cncf.io/api/ds/query", "application/json", requestBody)
4950
if err != nil {
50-
return err, nil
51+
return nil, err
5152
}
5253
defer resp.Body.Close()
5354

5455
if resp.StatusCode != http.StatusOK {
55-
return errors.Wrap(err, "bad error code from devstats : "+string(resp.StatusCode)), nil
56+
return nil, errors.Wrap(err, fmt.Sprintf("bad error code from devstats: %d", resp.StatusCode))
5657
}
5758

5859
var parsed map[string]map[string]map[string][]Frames
5960
body, err := ioutil.ReadAll(resp.Body)
6061
if err != nil {
61-
return err, nil
62+
return nil, err
6263
}
6364
err = json.Unmarshal(body, &parsed)
6465
if err != nil {
65-
return errors.Wrap(err, "unable to parse json from devstats"), nil
66+
return nil, errors.Wrap(err, "unable to parse json from devstats")
6667
}
6768

6869
foo := parsed["results"]["A"]["frames"][0].Data.Items[0]
@@ -72,5 +73,5 @@ func GetContributionsForAYear(repository string, period string) (error, []Contri
7273
for i := 0; i < len(foo); i++ {
7374
contribs = append(contribs, Contribution{foo[i].(string), "", int(bar[i].(float64)), -1})
7475
}
75-
return nil, contribs
76+
return contribs, nil
7677
}

pkg/utils/file_utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func GetOwnerFiles(root string) ([]string, error) {
9494
if info.IsDir() {
9595
return nil
9696
}
97-
if "OWNERS" == filepath.Base(path) && !strings.Contains(path, "vendor") {
97+
if filepath.Base(path) == "OWNERS" && !strings.Contains(path, "vendor") {
9898
matches = append(matches, path)
9999
}
100100
return nil

pkg/utils/github_utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ func GetKubernetesOwnersFiles() (*[]string, error) {
103103
directories := make([]string, len(c.Files))
104104
for _, directory := range c.Files {
105105
if len(directory.Path) > 0 &&
106-
strings.Index(directory.Path, "/OWNERS") != -1 &&
106+
strings.Contains(directory.Path, "/OWNERS") &&
107107
strings.Index(directory.Path, "vendor/") != 0 {
108108
directories = append(directories, directory.Path)
109109
}

pkg/utils/yaml_utils.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func switchToEmeritus(rootNode *yaml3.Node, user string) {
8080
func addUserToEmeritusList(emeritusSeqNode *yaml3.Node, user string) {
8181
foundInEmeritusList := false
8282
for _, item := range emeritusSeqNode.Content {
83-
if item.Kind == yaml3.ScalarNode && strings.ToLower(item.Value) == strings.ToLower(user) {
83+
if item.Kind == yaml3.ScalarNode && strings.EqualFold(item.Value, user) {
8484
foundInEmeritusList = true
8585
}
8686
}
@@ -169,7 +169,7 @@ func removeUserFromApproversAndReviewers(mappingNode *yaml3.Node, user string) b
169169
var newList []*yaml3.Node
170170
for _, item := range seqNode.Content {
171171
// skip the user we want to eliminate from the list
172-
if item.Kind == yaml3.ScalarNode && strings.ToLower(item.Value) == strings.ToLower(user) {
172+
if item.Kind == yaml3.ScalarNode && strings.EqualFold(item.Value, user) {
173173
if node.Value == "approvers" {
174174
foundInApproverList = true
175175
}

0 commit comments

Comments
 (0)