community: cleanup code, reduce complexity, add make targets

.golangci.yaml

@@ -0,0 +1,6 @@
+linters:
+  enable:
+  - gocyclo
+linters-settings:
+  gocyclo:
+    min-complexity: 10

Makefile

@@ -1,7 +1,40 @@
+.PHONY: generate validate-sigs test coverage lint
+
+export GOLANGCI_LINT_VERSION := v1.62.2
+ifndef GOPATH
+    GOPATH=$(shell go env GOPATH)
+    export GOPATH
+endif
+WORKDIR := /tmp
+LOCAL_BIN := $(WORKDIR)/local_bin
+PATH := $(LOCAL_BIN):${PATH}
+ifndef ARTIFACTS
+	export ARTIFACTS := $(WORKDIR)/artifacts
+endif
+ifndef COVERAGE_OUTPUT_PATH
+	COVERAGE_OUTPUT_PATH := ${ARTIFACTS}/coverage.html
+endif
+
+work-dirs:
+	mkdir -p ${ARTIFACTS}
+	mkdir -p ${LOCAL_BIN}
+
 generate:
 	go run ./validators/cmd/sigs --dry-run=false
 	go run ./generators/cmd/sigs
 	go run ./generators/cmd/alumni
 
 validate-sigs:
 	go run ./validators/cmd/sigs
+
+test:
+	go test ./...
+
+coverage: work-dirs
+	if ! command -V covreport; then GOBIN=$(LOCAL_BIN) go install github.com/cancue/covreport@latest; fi
+	go test ./... -coverprofile=/tmp/coverage.out
+	covreport -i /tmp/coverage.out -o $(COVERAGE_OUTPUT_PATH)
+
+lint: work-dirs
+	if ! command -V golangci-lint; then curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b "${LOCAL_BIN}" ${GOLANGCI_LINT_VERSION} ; fi
+	golangci-lint run --verbose

generators/cmd/contributions/main.go

@@ -34,7 +34,7 @@ import (
 	"strings"
 )
 
-type ContributionReportOptions struct {
+type contributionReportOptions struct {
 	Org                   string `yaml:"org"`
 	Repo                  string `yaml:"repo"`
 	Username              string `yaml:"username"`
@@ -47,16 +47,49 @@ type ContributionReportOptions struct {
 	OwnersAliasesFilePath string `yaml:"ownersAliasesFilePath"`
 }
 
-type SkipInactiveCheckConfig struct {
+func (o *contributionReportOptions) defaultOwnersAliasesPath() string {
+	return filepath.Join(filepath.Dir(o.OwnersFilePath), "OWNERS_ALIASES")
+}
+
+func (o *contributionReportOptions) ownersAliasesFilePath() string {
+	ownersAliasesPath := o.defaultOwnersAliasesPath()
+	if o.OwnersAliasesFilePath != "" {
+		ownersAliasesPath = o.OwnersAliasesFilePath
+	}
+	return ownersAliasesPath
+}
+
+func (o *contributionReportOptions) validate() error {
+	if o.Username != "" {
+		log.Infof("creating report for user %q", o.Username)
+	} else if o.OrgsConfigFilePath == "" && o.OwnersFilePath == "" {
+		return fmt.Errorf("username or orgs-config-file-path or owners-file-path is required")
+	}
+	if o.GithubTokenPath == "" {
+		return fmt.Errorf("github token path is required")
+	}
+	return nil
+}
+
+func (o *contributionReportOptions) makeGeneratorOptions() contributions.ContributionReportGeneratorOptions {
+	return contributions.ContributionReportGeneratorOptions{
+		Org:             o.Org,
+		Repo:            o.Repo,
+		GithubTokenPath: o.GithubTokenPath,
+		Months:          o.Months,
+	}
+}
+
+type skipInactiveCheckConfig struct {
 	Name   string   `yaml:"name"`
 	Github []string `yaml:"github"`
 }
 
-type ContributionReportConfig struct {
-	SkipInactive map[string][]SkipInactiveCheckConfig `yaml:"skipInactive"`
+type contributionReportConfig struct {
+	SkipInactive map[string][]skipInactiveCheckConfig `yaml:"skipInactive"`
 }
 
-func (c *ContributionReportConfig) ShouldSkip(org, repo, userName string) (bool, string) {
+func (c *contributionReportConfig) ShouldSkip(org, repo, userName string) (bool, string) {
 	var skipInactiveKey string
 	if repo != "" {
 		skipInactiveKey = fmt.Sprintf("%s/%s", org, repo)
@@ -69,7 +102,7 @@ func (c *ContributionReportConfig) ShouldSkip(org, repo, userName string) (bool,
 	}
 	for _, config := range configs {
 		for _, github := range config.Github {
-			if strings.ToLower(userName) == strings.ToLower(github) {
+			if strings.EqualFold(userName, github) {
 				return true, config.Name
 			}
 		}
@@ -81,32 +114,11 @@ var (
 	//go:embed default-config.yaml
 	defaultConfigContent []byte
 
-	defaultConfig *ContributionReportConfig
+	defaultConfig *contributionReportConfig
 )
 
-func (o ContributionReportOptions) validate() error {
-	if o.Username != "" {
-		log.Infof("creating report for user %q", o.Username)
-	} else if o.OrgsConfigFilePath == "" && o.OwnersFilePath == "" {
-		return fmt.Errorf("username or orgs-config-file-path or owners-file-path is required")
-	}
-	if o.GithubTokenPath == "" {
-		return fmt.Errorf("github token path is required")
-	}
-	return nil
-}
-
-func (o ContributionReportOptions) makeGeneratorOptions() contributions.ContributionReportGeneratorOptions {
-	return contributions.ContributionReportGeneratorOptions{
-		Org:             o.Org,
-		Repo:            o.Repo,
-		GithubTokenPath: o.GithubTokenPath,
-		Months:          o.Months,
-	}
-}
-
-func gatherContributionReportOptions() (*ContributionReportOptions, error) {
-	o := ContributionReportOptions{}
+func gatherContributionReportOptions() (*contributionReportOptions, error) {
+	o := contributionReportOptions{}
 	fs := flag.NewFlagSet(os.Args[0], flag.ExitOnError)
 	fs.StringVar(&o.Org, "org", "kubevirt", "org name")
 	fs.StringVar(&o.Repo, "repo", "", "repo name (leave empty to create an org activity report)")
@@ -133,101 +145,135 @@ func init() {
 }
 
 func main() {
-	contributionReportOptions, err := gatherContributionReportOptions()
+	contributionReportOpts, err := gatherContributionReportOptions()
 	if err != nil {
 		log.Fatalf("error parsing arguments %v: %v", os.Args[1:], err)
 	}
-	if err = contributionReportOptions.validate(); err != nil {
+	if err = contributionReportOpts.validate(); err != nil {
 		log.Fatalf("error validating arguments: %v", err)
 	}
 
-	generator, err := contributions.NewContributionReportGenerator(contributionReportOptions.makeGeneratorOptions())
+	generator, err := contributions.NewContributionReportGenerator(contributionReportOpts.makeGeneratorOptions())
 	if err != nil {
 		log.Fatalf("failed to create report generator: %v", err)
 	}
 
-	reporter := NewDefaultReporter(contributionReportOptions, defaultConfig)
-	userNames := []string{contributionReportOptions.Username}
-	if contributionReportOptions.Username == "" {
-		if !contributionReportOptions.ReportAll {
-			reporter = NewInactiveOnlyReporter(contributionReportOptions, defaultConfig)
+	newCommunityReportGenerator(contributionReportOpts, generator).generateRequestedCommunityReport()
+}
+
+func newCommunityReportGenerator(contributionReportOpts *contributionReportOptions, generator *contributions.ContributionReportGenerator) *communityReportGenerator {
+	communityReportGen := &communityReportGenerator{
+		contributionReportOpts:      contributionReportOpts,
+		contributionReportGenerator: generator,
+	}
+	return communityReportGen
+}
+
+type communityReportGenerator struct {
+	contributionReportOpts      *contributionReportOptions
+	contributionReportGenerator *contributions.ContributionReportGenerator
+	reporter                    Reporter
+	userNames                   []string
+}
+
+func (g *communityReportGenerator) generateRequestedCommunityReport() {
+	g.determineReporterAndUserNames()
+	g.generateReportPerUser()
+	g.printReportSummary()
+	g.handleReportOutput()
+}
+
+func (g *communityReportGenerator) determineReporterAndUserNames() {
+	g.reporter = NewDefaultReporter(g.contributionReportOpts, defaultConfig)
+	g.userNames = []string{g.contributionReportOpts.Username}
+	if g.contributionReportOpts.Username != "" {
+		return
+	}
+
+	if !g.contributionReportOpts.ReportAll {
+		g.reporter = NewInactiveOnlyReporter(g.contributionReportOpts, defaultConfig)
+	}
+
+	if g.contributionReportOpts.OwnersFilePath != "" {
+		ownersYAML, err := owners.ReadFile(g.contributionReportOpts.OwnersFilePath)
+		if err != nil {
+			log.Fatalf("invalid arguments: %v", err)
 		}
-		if contributionReportOptions.OwnersFilePath != "" {
-			ownersYAML, err := owners.ReadFile(contributionReportOptions.OwnersFilePath)
-			if err != nil {
-				log.Fatalf("invalid arguments: %v", err)
-			}
-			ownersAliasesPath := defaultOwnersAsiasesPath(contributionReportOptions)
-			if contributionReportOptions.OwnersAliasesFilePath != "" {
-				ownersAliasesPath = contributionReportOptions.OwnersAliasesFilePath
-			}
-			stat, err := os.Stat(ownersAliasesPath)
-			ownersAliases := &owners.OwnersAliases{}
-			if err == nil && !stat.IsDir() {
-				ownersAliases, err = owners.ReadAliasesFile(ownersAliasesPath)
-				if err != nil {
-					log.Fatalf("invalid aliases file %q: %v", ownersAliasesPath, err)
-				}
-			}
-			userNames = ownersYAML.AllReviewers()
-			userNames = append(userNames, ownersYAML.AllApprovers()...)
-			userNames = ownersAliases.Resolve(userNames)
-			userNames = uniq(userNames)
-			sort.Strings(userNames)
-		} else if contributionReportOptions.OrgsConfigFilePath != "" {
-			orgsYAML, err := orgs.ReadFile(contributionReportOptions.OrgsConfigFilePath)
+		g.userNames = ownersYAML.AllReviewers()
+		g.userNames = append(g.userNames, ownersYAML.AllApprovers()...)
+
+		ownersAliasesPath := g.contributionReportOpts.ownersAliasesFilePath()
+		stat, err := os.Stat(ownersAliasesPath)
+		ownersAliases := &owners.OwnersAliases{}
+		if err == nil && !stat.IsDir() {
+			ownersAliases, err = owners.ReadAliasesFile(ownersAliasesPath)
 			if err != nil {
-				log.Fatalf("invalid arguments: %v", err)
+				log.Fatalf("invalid aliases file %q: %v", ownersAliasesPath, err)
 			}
-			userNames = orgsYAML.Orgs[contributionReportOptions.Org].Members
 		}
+		g.userNames = ownersAliases.Resolve(g.userNames)
+		g.userNames = uniq(g.userNames)
+		sort.Strings(g.userNames)
+	} else if g.contributionReportOpts.OrgsConfigFilePath != "" {
+		orgsYAML, err := orgs.ReadFile(g.contributionReportOpts.OrgsConfigFilePath)
+		if err != nil {
+			log.Fatalf("invalid arguments: %v", err)
+		}
+		g.userNames = orgsYAML.Orgs[g.contributionReportOpts.Org].Members
+	}
+}
+
+func uniq(elements ...[]string) []string {
+	uniqMap := make(map[string]struct{})
+	for _, values := range elements {
+		for _, value := range values {
+			uniqMap[value] = struct{}{}
+		}
+	}
+	var uniqueValues []string
+	for uniqueValue := range uniqMap {
+		uniqueValues = append(uniqueValues, uniqueValue)
 	}
+	return uniqueValues
+}
 
-	for _, userName := range userNames {
-		if contributionReportOptions.Username == "" {
-			shouldSkip, reason := defaultConfig.ShouldSkip(contributionReportOptions.Org, contributionReportOptions.Repo, userName)
+func (g *communityReportGenerator) generateReportPerUser() {
+	for _, userName := range g.userNames {
+		if g.contributionReportOpts.Username == "" {
+			shouldSkip, reason := defaultConfig.ShouldSkip(g.contributionReportOpts.Org, g.contributionReportOpts.Repo, userName)
 			if shouldSkip {
 				log.Debugf("skipping user %s (reason: %s)", userName, reason)
-				reporter.Skip(userName, reason)
+				g.reporter.Skip(userName, reason)
 				continue
 			}
 		}
-		activity, err := generator.GenerateReport(userName)
+		activity, err := g.contributionReportGenerator.GenerateReport(userName)
 		if err != nil {
 			log.Fatalf("failed to generate report: %v", err)
 		}
-		err = reporter.Report(activity, userName)
+		err = g.reporter.Report(activity, userName)
 		if err != nil {
 			log.Fatalf("failed to report: %v", err)
 		}
 	}
-	fmt.Printf(reporter.Summary())
-	if contributionReportOptions.ReportOutputFilePath != "" {
-		reportBytes, err := yaml.Marshal(reporter.Full())
+}
+
+func (g *communityReportGenerator) printReportSummary() {
+	_, err := fmt.Print(g.reporter.Summary())
+	if err != nil {
+		log.Fatalf("failed to print report summary: %v", err)
+	}
+}
+
+func (g *communityReportGenerator) handleReportOutput() {
+	if g.contributionReportOpts.ReportOutputFilePath != "" {
+		reportBytes, err := yaml.Marshal(g.reporter.Full())
 		if err != nil {
 			log.Fatalf("failed to write report: %v", err)
 		}
-		err = os.WriteFile(contributionReportOptions.ReportOutputFilePath, reportBytes, 0666)
+		err = os.WriteFile(g.contributionReportOpts.ReportOutputFilePath, reportBytes, 0666)
 		if err != nil {
 			log.Fatalf("failed to write report: %v", err)
 		}
 	}
 }
-
-func defaultOwnersAsiasesPath(contributionReportOptions *ContributionReportOptions) string {
-	return filepath.Join(filepath.Dir(contributionReportOptions.OwnersFilePath), "OWNERS_ALIASES")
-}
-
-func uniq(elements ...[]string) []string {
-	uniqMap := make(map[string]struct{})
-	for _, values := range elements {
-		for _, value := range values {
-			uniqMap[value] = struct{}{}
-		}
-	}
-	var uniqueValues []string
-	for uniqueValue := range uniqMap {
-		uniqueValues = append(uniqueValues, uniqueValue)
-	}
-	return uniqueValues
-}

generators/cmd/contributions/report.go

@@ -40,13 +40,13 @@ func (receiver *ReportResult) SkipUser(reason, userName string) {
 }
 
 type Report struct {
-	ReportOptions *ContributionReportOptions `yaml:"reportOptions"`
-	ReportConfig  *ContributionReportConfig  `yaml:"reportConfig"`
+	ReportOptions *contributionReportOptions `yaml:"reportOptions"`
+	ReportConfig  *contributionReportConfig  `yaml:"reportConfig"`
 	Result        *ReportResult              `yaml:"result"`
 	Log           []string                   `yaml:"log"`
 }
 
-func NewReportWithConfiguration(options *ContributionReportOptions, config *ContributionReportConfig) *Report {
+func NewReportWithConfiguration(options *contributionReportOptions, config *contributionReportConfig) *Report {
 	return &Report{
 		ReportConfig:  config,
 		ReportOptions: options,
@@ -65,7 +65,7 @@ type DefaultReporter struct {
 	report *Report
 }
 
-func NewDefaultReporter(options *ContributionReportOptions, config *ContributionReportConfig) Reporter {
+func NewDefaultReporter(options *contributionReportOptions, config *contributionReportConfig) Reporter {
 	d := &DefaultReporter{}
 	d.report = NewReportWithConfiguration(options, config)
 	return d
@@ -76,7 +76,7 @@ func (d *DefaultReporter) Skip(userName string, reason string) {
 }
 
 func (d *DefaultReporter) Report(r contributions.ContributionReport, userName string) error {
-	fmt.Printf(r.Summary())
+	fmt.Print(r.Summary())
 	_, err := r.WriteToFile("/tmp", userName)
 	if err != nil {
 		return fmt.Errorf("failed to write file: %v", err)
@@ -100,7 +100,7 @@ func (d *InactiveOnlyReporter) Skip(userName string, reason string) {
 	d.report.Result.SkipUser(reason, userName)
 }
 
-func NewInactiveOnlyReporter(options *ContributionReportOptions, config *ContributionReportConfig) Reporter {
+func NewInactiveOnlyReporter(options *contributionReportOptions, config *contributionReportConfig) Reporter {
 	i := &InactiveOnlyReporter{}
 	i.report = NewReportWithConfiguration(options, config)
 	return i