Skip to content

Commit

Permalink
Address warnings and minor cleanups
Browse files Browse the repository at this point in the history
* 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 <madhav.jiv@gmail.com>
  • Loading branch information
MadhavJivrajani authored and dims committed Feb 17, 2022
1 parent 78c7c69 commit 023dd3e
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 26 deletions.
7 changes: 3 additions & 4 deletions cmd/audit.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package cmd

import (
"fmt"
"github.com/pkg/errors"
"io/ioutil"
"net/http"
"os"
Expand All @@ -30,6 +29,8 @@ import (
"sync"
"time"

"github.com/pkg/errors"

"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/util/sets"

Expand Down Expand Up @@ -96,13 +97,11 @@ func auditLocalOwnersFiles(context *utils.Context, args []string) {
for _, groups := range context.PrefixToGroupMap() {
for _, group := range groups {
listOfGroups = append(listOfGroups, group.Dir)
var files []string
for _, subproject := range group.Subprojects {
for _, owner := range subproject.Owners {
if strings.Contains(owner, "/kubernetes/kubernetes/") {
split := strings.SplitN(owner, "/", 7)
filename := split[len(split)-1]
files = append(files, filename)
if val, ok := mapFilesToGroups[filename]; ok {
val.Insert(group.Dir)
} else {
Expand Down Expand Up @@ -172,7 +171,7 @@ func auditLocalOwnersFiles(context *utils.Context, args []string) {
}
}
for _, line := range infoLog.List() {
fmt.Printf(line)
fmt.Println(line)
}
}

Expand Down
5 changes: 3 additions & 2 deletions cmd/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ package cmd

import (
"fmt"
"github.com/spf13/cobra"
"os"
"sort"
"strings"
"time"

"github.com/spf13/cobra"

"k8s.io/apimachinery/pkg/util/sets"

"github.com/dims/maintainers/pkg/utils"
Expand Down Expand Up @@ -91,7 +92,7 @@ func exportOwnersAndAliases(pwd string) error {
}
userIDs.Insert(configOwners.Approvers...)
userIDs.Insert(configOwners.Reviewers...)
for key, _ := range repoAliases {
for key := range repoAliases {
if userIDs.Has(key) {
userIDs.Delete(key)
aliases.Insert(key)
Expand Down
29 changes: 22 additions & 7 deletions cmd/prune.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ package cmd

import (
"fmt"
"github.com/spf13/cobra"
"os"
"path/filepath"
"sort"
"strings"
"time"

"github.com/spf13/cobra"

"k8s.io/apimachinery/pkg/util/sets"

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

if !skipDS {
err, contribs := utils.GetContributionsForAYear(repositoryDS, periodDS)
contribs, err := utils.GetContributionsForAYear(repositoryDS, periodDS)
if err != nil {
panic(err)
}
if contribs == nil || len(contribs) == 0 {
if len(contribs) == 0 {
panic("unable to find any contributions in repository : " + repositoryDS)
}
for _, id := range uniqueUsers {
for _, item := range contribs {
if strings.ToLower(item.ID) == strings.ToLower(id) {
ownerContribs = append(ownerContribs, utils.Contribution{id, item.ID, item.ContribCount, -1})
if strings.EqualFold(item.ID, id) {
ownerContribs = append(ownerContribs,
utils.Contribution{
ID: id,
Alias: item.ID,
ContribCount: item.ContribCount,
CommentCount: -1,
},
)
userIDs.Delete(id)
break
}
}
}
} else {
for _, id := range uniqueUsers {
ownerContribs = append(ownerContribs, utils.Contribution{id, id, -1, -1})
ownerContribs = append(ownerContribs,
utils.Contribution{
ID: id,
Alias: id,
ContribCount: -1,
CommentCount: -1,
},
)
userIDs.Delete(id)
}
}
Expand Down Expand Up @@ -233,7 +248,7 @@ func getOwnersAndAliases(pwd string) (sets.String, map[string][]string, []string
userIDs.Insert(configOwners.Reviewers...)
}

for key, _ := range repoAliases {
for key := range repoAliases {
userIDs.Delete(key)
}
if len(aliasPath) > 0 {
Expand Down
4 changes: 2 additions & 2 deletions cmd/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ var validateCmd = &cobra.Command{
errors2 := warnFileMismatchesBetweenKubernetesRepoAndSigsYaml(fileMap)
errors = append(errors, errors2...)

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

for key, val := range fileMap {
if strings.Index(key, "kubernetes/kubernetes") == -1 {
if !strings.Contains(key, "kubernetes/kubernetes") {
continue
}
found := false
Expand Down
15 changes: 8 additions & 7 deletions pkg/utils/devstats_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@ import (
"bytes"
"encoding/json"
"fmt"
"github.com/pkg/errors"
"io/ioutil"
"net/http"
"strings"

"github.com/pkg/errors"
)

func GetContributionsForAYear(repository string, period string) (error, []Contribution) {
func GetContributionsForAYear(repository string, period string) ([]Contribution, error) {
postBody := `{
"queries": [{
"refId": "A",
Expand All @@ -47,22 +48,22 @@ func GetContributionsForAYear(repository string, period string) (error, []Contri
requestBody := bytes.NewBuffer([]byte(postBody))
resp, err := http.Post("https://k8s.devstats.cncf.io/api/ds/query", "application/json", requestBody)
if err != nil {
return err, nil
return nil, err
}
defer resp.Body.Close()

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

var parsed map[string]map[string]map[string][]Frames
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
return err, nil
return nil, err
}
err = json.Unmarshal(body, &parsed)
if err != nil {
return errors.Wrap(err, "unable to parse json from devstats"), nil
return nil, errors.Wrap(err, "unable to parse json from devstats")
}

foo := parsed["results"]["A"]["frames"][0].Data.Items[0]
Expand All @@ -72,5 +73,5 @@ func GetContributionsForAYear(repository string, period string) (error, []Contri
for i := 0; i < len(foo); i++ {
contribs = append(contribs, Contribution{foo[i].(string), "", int(bar[i].(float64)), -1})
}
return nil, contribs
return contribs, nil
}
2 changes: 1 addition & 1 deletion pkg/utils/file_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func GetOwnerFiles(root string) ([]string, error) {
if info.IsDir() {
return nil
}
if "OWNERS" == filepath.Base(path) && !strings.Contains(path, "vendor") {
if filepath.Base(path) == "OWNERS" && !strings.Contains(path, "vendor") {
matches = append(matches, path)
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/github_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func GetKubernetesOwnersFiles() (*[]string, error) {
directories := make([]string, len(c.Files))
for _, directory := range c.Files {
if len(directory.Path) > 0 &&
strings.Index(directory.Path, "/OWNERS") != -1 &&
strings.Contains(directory.Path, "/OWNERS") &&
strings.Index(directory.Path, "vendor/") != 0 {
directories = append(directories, directory.Path)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/utils/yaml_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func switchToEmeritus(rootNode *yaml3.Node, user string) {
func addUserToEmeritusList(emeritusSeqNode *yaml3.Node, user string) {
foundInEmeritusList := false
for _, item := range emeritusSeqNode.Content {
if item.Kind == yaml3.ScalarNode && strings.ToLower(item.Value) == strings.ToLower(user) {
if item.Kind == yaml3.ScalarNode && strings.EqualFold(item.Value, user) {
foundInEmeritusList = true
}
}
Expand Down Expand Up @@ -169,7 +169,7 @@ func removeUserFromApproversAndReviewers(mappingNode *yaml3.Node, user string) b
var newList []*yaml3.Node
for _, item := range seqNode.Content {
// skip the user we want to eliminate from the list
if item.Kind == yaml3.ScalarNode && strings.ToLower(item.Value) == strings.ToLower(user) {
if item.Kind == yaml3.ScalarNode && strings.EqualFold(item.Value, user) {
if node.Value == "approvers" {
foundInApproverList = true
}
Expand Down

0 comments on commit 023dd3e

Please sign in to comment.