From c8729eb15c6c92f0afcb3308570b40350203857d Mon Sep 17 00:00:00 2001 From: Daniel Hiller Date: Wed, 11 Dec 2024 13:26:17 +0100 Subject: [PATCH 01/11] lint: add make target Adds a make target `lint` that will be used to run enabled linters, which are configured via .golangci.yaml . Initially only the default linters are enabled, additionally the gocyclo linter with ccn > 10. Signed-off-by: Daniel Hiller --- .golangci.yaml | 6 ++++++ Makefile | 14 ++++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 .golangci.yaml diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 00000000..6b266392 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,6 @@ +linters: + enable: + - gocyclo +linters-settings: + gocyclo: + min-complexity: 10 \ No newline at end of file diff --git a/Makefile b/Makefile index 418cbf16..3f67dacc 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,11 @@ +.PHONY: generate validate-sigs install-metrics-binaries lint + +export GOLANGCI_LINT_VERSION := v1.62.2 +ifndef $(GOPATH) + GOPATH=$(shell go env GOPATH) + export GOPATH +endif + generate: go run ./validators/cmd/sigs --dry-run=false go run ./generators/cmd/sigs @@ -5,3 +13,9 @@ generate: validate-sigs: go run ./validators/cmd/sigs + +install-metrics-binaries: + if ! command -V golangci-lint; then curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b ${GOPATH}/bin ${GOLANGCI_LINT_VERSION} ; fi + +lint: install-metrics-binaries + golangci-lint run --verbose From 5d04c82b1089353cfb9cc351ad035f092e2dd179 Mon Sep 17 00:00:00 2001 From: Daniel Hiller Date: Thu, 12 Dec 2024 13:37:05 +0100 Subject: [PATCH 02/11] lint: fix deprecated io/ioutil usage Signed-off-by: Daniel Hiller --- validators/cmd/sigs/sigs-validator.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/validators/cmd/sigs/sigs-validator.go b/validators/cmd/sigs/sigs-validator.go index f71d30e8..e43ef818 100644 --- a/validators/cmd/sigs/sigs-validator.go +++ b/validators/cmd/sigs/sigs-validator.go @@ -3,7 +3,6 @@ package main import ( "flag" "fmt" - "io/ioutil" "kubevirt.io/community/pkg/labels" "kubevirt.io/community/pkg/orgs" "kubevirt.io/community/pkg/sigs" @@ -105,7 +104,7 @@ func main() { if err != nil { log.Fatalf("stat for file %q failed: %v", opts.sigsFilePath, err) } - err = ioutil.WriteFile(opts.sigsFilePath, output, stat.Mode()) + err = os.WriteFile(opts.sigsFilePath, output, stat.Mode()) if err != nil { log.Fatalf("write to file %q failed: %v", opts.sigsFilePath, err) } From 8a5575ee498c86bb1856464eda75dfaca869708c Mon Sep 17 00:00:00 2001 From: Daniel Hiller Date: Wed, 11 Dec 2024 13:28:54 +0100 Subject: [PATCH 03/11] sigs-validator,refactor: extract funcs to reduce ccn Signed-off-by: Daniel Hiller --- validators/cmd/sigs/sigs-validator.go | 57 +++++++++++++++++---------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/validators/cmd/sigs/sigs-validator.go b/validators/cmd/sigs/sigs-validator.go index e43ef818..d87b3def 100644 --- a/validators/cmd/sigs/sigs-validator.go +++ b/validators/cmd/sigs/sigs-validator.go @@ -74,21 +74,7 @@ func main() { kubevirtOrg := orgsYAML.Orgs["kubevirt"] - for _, sig := range sigsYAML.Sigs { - validateGroup("sig", sig, labelsYAML, kubevirtOrg) - } - - for _, wg := range sigsYAML.Workinggroups { - validateGroup("wg", wg, labelsYAML, kubevirtOrg) - } - - for _, ug := range sigsYAML.Usergroups { - validateGroup("ug", ug, labelsYAML, kubevirtOrg) - } - - for _, committee := range sigsYAML.Committees { - validateGroup("committee", committee, labelsYAML, kubevirtOrg) - } + validateGroups(sigsYAML, labelsYAML, kubevirtOrg) output, err := yaml.Marshal(sigsYAML) if err != nil { @@ -112,10 +98,34 @@ func main() { } +func validateGroups(sigsYAML *sigs.Sigs, labelsYAML *labels.LabelsYAML, kubevirtOrg orgs.Org) { + for _, sig := range sigsYAML.Sigs { + validateGroup("sig", sig, labelsYAML, kubevirtOrg) + } + + for _, wg := range sigsYAML.Workinggroups { + validateGroup("wg", wg, labelsYAML, kubevirtOrg) + } + + for _, ug := range sigsYAML.Usergroups { + validateGroup("ug", ug, labelsYAML, kubevirtOrg) + } + + for _, committee := range sigsYAML.Committees { + validateGroup("committee", committee, labelsYAML, kubevirtOrg) + } +} + func validateGroup(groupType string, groupToValidate *sigs.Group, labelsYAML *labels.LabelsYAML, kubevirtOrg orgs.Org) { groupLog := log.WithField(groupType, groupToValidate.Name) + validateDirectoryExists(groupToValidate, groupLog) + validateLabelExists(groupToValidate, labelsYAML, groupLog) + validateLeads(groupToValidate, kubevirtOrg, groupLog) + validateChairs(groupToValidate, kubevirtOrg, groupLog) + validateSubprojects(groupToValidate, groupLog, kubevirtOrg) +} - // check dir exists +func validateDirectoryExists(groupToValidate *sigs.Group, groupLog *log.Entry) { if groupToValidate.Dir != "" { stat, err := os.Stat(groupToValidate.Dir) if err != nil { @@ -126,8 +136,9 @@ func validateGroup(groupType string, groupToValidate *sigs.Group, labelsYAML *la groupToValidate.Dir = "" } } +} - // check label exists +func validateLabelExists(groupToValidate *sigs.Group, labelsYAML *labels.LabelsYAML, groupLog *log.Entry) { if groupToValidate.Label != "" { foundLabel := false for _, label := range labelsYAML.Default.Labels { @@ -141,8 +152,10 @@ func validateGroup(groupType string, groupToValidate *sigs.Group, labelsYAML *la groupToValidate.Label = "" } } +} - // check leads - github handles are part of org +// validateLeads checks that leads github handles are part of org, removes all that don't satisfy this requirement +func validateLeads(groupToValidate *sigs.Group, kubevirtOrg orgs.Org, groupLog *log.Entry) { var checkedMembers []*sigs.Lead for _, orgMember := range groupToValidate.Leads { if !kubevirtOrg.HasMember(orgMember.Github) { @@ -152,8 +165,10 @@ func validateGroup(groupType string, groupToValidate *sigs.Group, labelsYAML *la } } groupToValidate.Leads = checkedMembers +} - // check chairs - github handles are part of org +// validateLeads checks that chairs github handles are part of org, removes all that don't satisfy this +func validateChairs(groupToValidate *sigs.Group, kubevirtOrg orgs.Org, groupLog *log.Entry) { if groupToValidate.Leadership != nil { var checkedLeadership []*sigs.Chair for _, orgMember := range groupToValidate.Leadership.Chairs { @@ -165,8 +180,9 @@ func validateGroup(groupType string, groupToValidate *sigs.Group, labelsYAML *la } groupToValidate.Leadership.Chairs = checkedLeadership } +} - // check subprojects +func validateSubprojects(groupToValidate *sigs.Group, groupLog *log.Entry, kubevirtOrg orgs.Org) { for _, subProject := range groupToValidate.SubProjects { subprojectLog := groupLog.WithField("subproject", subProject.Name) foundOwners := validateOwnersReferences(subProject, subprojectLog) @@ -182,7 +198,6 @@ func validateGroup(groupType string, groupToValidate *sigs.Group, labelsYAML *la } } subProject.Leads = checkedSubprojectChairs - } } From 612090f1832a7ffc84437f8206a8afe9d725c488 Mon Sep 17 00:00:00 2001 From: Daniel Hiller Date: Wed, 11 Dec 2024 13:35:59 +0100 Subject: [PATCH 04/11] go,test: add make target Adds a `test` target to the Makefile that executes the go tests. Signed-off-by: Daniel Hiller --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index 3f67dacc..bf5675b8 100644 --- a/Makefile +++ b/Makefile @@ -14,6 +14,9 @@ generate: validate-sigs: go run ./validators/cmd/sigs +test: + go test ./... + install-metrics-binaries: if ! command -V golangci-lint; then curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b ${GOPATH}/bin ${GOLANGCI_LINT_VERSION} ; fi From 8477cf73190c536b67e82a795f60553f29621a08 Mon Sep 17 00:00:00 2001 From: Daniel Hiller Date: Wed, 11 Dec 2024 18:04:02 +0100 Subject: [PATCH 05/11] sigs-validator: add unit tests Adds unit tests for sig validation as precondition of refactoring. Signed-off-by: Daniel Hiller --- validators/cmd/sigs/sigs-validator.go | 2 +- validators/cmd/sigs/sigs-validator_test.go | 462 ++++++++++++++++++ .../cmd/sigs/testdata/existing-dir/.gitkeep | 0 validators/cmd/sigs/testdata/existing-file | 1 + 4 files changed, 464 insertions(+), 1 deletion(-) create mode 100644 validators/cmd/sigs/sigs-validator_test.go create mode 100644 validators/cmd/sigs/testdata/existing-dir/.gitkeep create mode 100644 validators/cmd/sigs/testdata/existing-file diff --git a/validators/cmd/sigs/sigs-validator.go b/validators/cmd/sigs/sigs-validator.go index d87b3def..7da0762c 100644 --- a/validators/cmd/sigs/sigs-validator.go +++ b/validators/cmd/sigs/sigs-validator.go @@ -202,7 +202,7 @@ func validateSubprojects(groupToValidate *sigs.Group, groupLog *log.Entry, kubev } func validateOwnersReferences(subProject *sigs.SubProject, subprojectLog *log.Entry) []string { - foundOwners := make([]string, 0) + var foundOwners []string for _, ownersFileURL := range subProject.Owners { response, err := http.DefaultClient.Head(ownersFileURL) if err != nil { diff --git a/validators/cmd/sigs/sigs-validator_test.go b/validators/cmd/sigs/sigs-validator_test.go new file mode 100644 index 00000000..b0e1ee6d --- /dev/null +++ b/validators/cmd/sigs/sigs-validator_test.go @@ -0,0 +1,462 @@ +/* + * This file is part of the KubeVirt project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Copyright the KubeVirt Authors. + * + */ + +package main + +import ( + log "github.com/sirupsen/logrus" + "gopkg.in/yaml.v3" + "kubevirt.io/community/pkg/labels" + "kubevirt.io/community/pkg/orgs" + "kubevirt.io/community/pkg/sigs" + "reflect" + "testing" +) + +func Test_validateGroups(t *testing.T) { + type args struct { + sigsYAML *sigs.Sigs + expectedSigsYAML *sigs.Sigs + labelsYAML *labels.LabelsYAML + kubevirtOrg orgs.Org + } + tests := []struct { + name string + args args + }{ + { + name: "sig: removes directory", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Dir: "non-existing-dir", + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + }, + }, + }, + }, + }, + { + name: "sig: leaves directory", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Dir: "testdata/existing-dir", + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Dir: "testdata/existing-dir", + }, + }, + }, + }, + }, + { + name: "sig: removes directory if not a dir", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Dir: "testdata/existing-file", + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + }, + }, + }, + }, + }, + { + name: "sig: removes chair", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Leadership: &sigs.Leadership{ + Chairs: []*sigs.Chair{ + {Github: "nonexisting-gh"}, + }, + }, + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Leadership: &sigs.Leadership{}, + }, + }, + }, + }, + }, + { + name: "sig: leaves chair", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Leadership: &sigs.Leadership{ + Chairs: []*sigs.Chair{ + {Github: "existing-gh"}, + }, + }, + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Leadership: &sigs.Leadership{ + Chairs: []*sigs.Chair{ + {Github: "existing-gh"}, + }, + }, + }, + }, + }, + kubevirtOrg: orgs.Org{ + Members: []string{ + "existing-gh", + }, + }, + }, + }, + { + name: "sig: removes lead", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Leads: []*sigs.Lead{ + { + Github: "nonexisting-gh", + }, + }, + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + }, + }, + }, + }, + }, + { + name: "sig: leaves lead", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Leads: []*sigs.Lead{ + { + Github: "existing-gh", + }, + }, + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Leads: []*sigs.Lead{ + { + Github: "existing-gh", + }, + }, + }, + }, + }, + kubevirtOrg: orgs.Org{ + Members: []string{ + "existing-gh", + }, + }, + }, + }, + { + name: "sig: removes label", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Label: "nonexisting-label", + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + }, + }, + }, + labelsYAML: &labels.LabelsYAML{ + Default: &labels.Repo{Labels: []*labels.Label{}}, + }, + }, + }, + { + name: "sig: leaves default label", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Label: "existing-label", + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Label: "existing-label", + }, + }, + }, + labelsYAML: &labels.LabelsYAML{ + Default: &labels.Repo{Labels: []*labels.Label{{ + Name: "existing-label", + }}}, + }, + }, + }, + { + name: "sig: removes repo-specific label", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + Label: "existing-label", + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + }, + }, + }, + labelsYAML: &labels.LabelsYAML{ + Default: &labels.Repo{Labels: []*labels.Label{{}}}, + Repos: map[string]*labels.Repo{ + "somerepo": {Labels: []*labels.Label{ + { + Name: "existing-label", + }, + }}, + }, + }, + }, + }, + { + name: "sig: subproject - leaves existing owners references", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + SubProjects: []*sigs.SubProject{ + { + Name: "some-subproject", + Owners: []string{ + "https://raw.githubusercontent.com/kubevirt/community/main/OWNERS", + }, + }, + }, + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + SubProjects: []*sigs.SubProject{ + { + Name: "some-subproject", + Owners: []string{ + "https://raw.githubusercontent.com/kubevirt/community/main/OWNERS", + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "sig: subproject - remove owners reference if not found", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + SubProjects: []*sigs.SubProject{ + { + Name: "some-subproject", + Owners: []string{ + "https://raw.githubusercontent.com/kubevirt/non-existing-repo/main/OWNERS", + }, + }, + }, + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + SubProjects: []*sigs.SubProject{ + { + Name: "some-subproject", + }, + }, + }, + }, + }, + }, + }, + { + name: "sig: subproject - leaves lead", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + SubProjects: []*sigs.SubProject{ + { + Name: "some-subproject", + Leads: []*sigs.Lead{ + { + Github: "existing-gh", + }, + }, + }, + }, + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + SubProjects: []*sigs.SubProject{ + { + Name: "some-subproject", + Leads: []*sigs.Lead{ + { + Github: "existing-gh", + }, + }, + }, + }, + }, + }, + }, + kubevirtOrg: orgs.Org{ + Members: []string{ + "existing-gh", + }, + }, + }, + }, + { + name: "sig: subproject - removes lead if not org member", + args: args{ + sigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + SubProjects: []*sigs.SubProject{ + { + Name: "some-subproject", + Leads: []*sigs.Lead{ + { + Github: "existing-gh", + }, + }, + }, + }, + }, + }, + }, + expectedSigsYAML: &sigs.Sigs{ + Sigs: []*sigs.Group{ + { + Name: "sig-test", + SubProjects: []*sigs.SubProject{ + { + Name: "some-subproject", + }, + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validateGroups(tt.args.sigsYAML, tt.args.labelsYAML, tt.args.kubevirtOrg) + if !reflect.DeepEqual(tt.args.expectedSigsYAML, tt.args.sigsYAML) { + t.Errorf("sigs yaml:\n\ngot: %v\n\nwant: %v", printSigsYAML(tt.args.sigsYAML), printSigsYAML(tt.args.expectedSigsYAML)) + } + }) + } +} + +func printSigsYAML(s *sigs.Sigs) string { + out, err := yaml.Marshal(s) + if err != nil { + log.Fatalf("failed to print sigs yaml: %v", err) + } + return string(out) +} diff --git a/validators/cmd/sigs/testdata/existing-dir/.gitkeep b/validators/cmd/sigs/testdata/existing-dir/.gitkeep new file mode 100644 index 00000000..e69de29b diff --git a/validators/cmd/sigs/testdata/existing-file b/validators/cmd/sigs/testdata/existing-file new file mode 100644 index 00000000..89626620 --- /dev/null +++ b/validators/cmd/sigs/testdata/existing-file @@ -0,0 +1 @@ +whatever \ No newline at end of file From ebc83718defdf6b7ed6c31b458fbd55bae6c21ad Mon Sep 17 00:00:00 2001 From: Daniel Hiller Date: Wed, 11 Dec 2024 18:30:43 +0100 Subject: [PATCH 06/11] cleanup,refactor: sigs-validator * reduce complexity by converting all groups into a single map * replace if then with early return * extract validateLeads, use that in two places Signed-off-by: Daniel Hiller --- validators/cmd/sigs/sigs-validator.go | 128 +++++++++++++------------- 1 file changed, 62 insertions(+), 66 deletions(-) diff --git a/validators/cmd/sigs/sigs-validator.go b/validators/cmd/sigs/sigs-validator.go index 7da0762c..76210347 100644 --- a/validators/cmd/sigs/sigs-validator.go +++ b/validators/cmd/sigs/sigs-validator.go @@ -99,20 +99,17 @@ func main() { } func validateGroups(sigsYAML *sigs.Sigs, labelsYAML *labels.LabelsYAML, kubevirtOrg orgs.Org) { - for _, sig := range sigsYAML.Sigs { - validateGroup("sig", sig, labelsYAML, kubevirtOrg) + groupsToValidate := map[string][]*sigs.Group{ + "sig": sigsYAML.Sigs, + "wg": sigsYAML.Workinggroups, + "ug": sigsYAML.Usergroups, + "committee": sigsYAML.Committees, } - for _, wg := range sigsYAML.Workinggroups { - validateGroup("wg", wg, labelsYAML, kubevirtOrg) - } - - for _, ug := range sigsYAML.Usergroups { - validateGroup("ug", ug, labelsYAML, kubevirtOrg) - } - - for _, committee := range sigsYAML.Committees { - validateGroup("committee", committee, labelsYAML, kubevirtOrg) + for groupType, groups := range groupsToValidate { + for _, group := range groups { + validateGroup(groupType, group, labelsYAML, kubevirtOrg) + } } } @@ -120,85 +117,84 @@ func validateGroup(groupType string, groupToValidate *sigs.Group, labelsYAML *la groupLog := log.WithField(groupType, groupToValidate.Name) validateDirectoryExists(groupToValidate, groupLog) validateLabelExists(groupToValidate, labelsYAML, groupLog) - validateLeads(groupToValidate, kubevirtOrg, groupLog) + validateGroupLeads(groupToValidate, kubevirtOrg, groupLog) validateChairs(groupToValidate, kubevirtOrg, groupLog) validateSubprojects(groupToValidate, groupLog, kubevirtOrg) } func validateDirectoryExists(groupToValidate *sigs.Group, groupLog *log.Entry) { - if groupToValidate.Dir != "" { - stat, err := os.Stat(groupToValidate.Dir) - if err != nil { - groupLog.Errorf("dir %q not found: %v", groupToValidate.Dir, err) - groupToValidate.Dir = "" - } else if !stat.IsDir() { - groupLog.Errorf("dir %q is not a directory", groupToValidate.Dir) - groupToValidate.Dir = "" - } + if groupToValidate.Dir == "" { + return + } + + stat, err := os.Stat(groupToValidate.Dir) + if err != nil { + groupLog.Errorf("dir %q not found: %v", groupToValidate.Dir, err) + groupToValidate.Dir = "" + } else if !stat.IsDir() { + groupLog.Errorf("dir %q is not a directory", groupToValidate.Dir) + groupToValidate.Dir = "" } } func validateLabelExists(groupToValidate *sigs.Group, labelsYAML *labels.LabelsYAML, groupLog *log.Entry) { - if groupToValidate.Label != "" { - foundLabel := false - for _, label := range labelsYAML.Default.Labels { - if label.Name == groupToValidate.Label { - foundLabel = true - break - } - } - if !foundLabel { - groupLog.Errorf("label %q not found", groupToValidate.Label) - groupToValidate.Label = "" - } + if groupToValidate.Label == "" { + return } -} -// validateLeads checks that leads github handles are part of org, removes all that don't satisfy this requirement -func validateLeads(groupToValidate *sigs.Group, kubevirtOrg orgs.Org, groupLog *log.Entry) { - var checkedMembers []*sigs.Lead - for _, orgMember := range groupToValidate.Leads { - if !kubevirtOrg.HasMember(orgMember.Github) { - groupLog.Errorf("lead %q not found", orgMember) - } else { - checkedMembers = append(checkedMembers, orgMember) + foundLabel := false + for _, label := range labelsYAML.Default.Labels { + if label.Name == groupToValidate.Label { + foundLabel = true + break } } - groupToValidate.Leads = checkedMembers + if !foundLabel { + groupLog.Errorf("label %q not found", groupToValidate.Label) + groupToValidate.Label = "" + } +} + +// validateGroupLeads checks that leads github handles are part of org, removes all that don't satisfy this requirement +func validateGroupLeads(groupToValidate *sigs.Group, kubevirtOrg orgs.Org, groupLog *log.Entry) { + groupToValidate.Leads = validateLeads(groupToValidate.Leads, kubevirtOrg, groupLog) } -// validateLeads checks that chairs github handles are part of org, removes all that don't satisfy this +// validateChairs checks that chairs github handles are part of org, removes all that don't satisfy this func validateChairs(groupToValidate *sigs.Group, kubevirtOrg orgs.Org, groupLog *log.Entry) { - if groupToValidate.Leadership != nil { - var checkedLeadership []*sigs.Chair - for _, orgMember := range groupToValidate.Leadership.Chairs { - if !kubevirtOrg.HasMember(orgMember.Github) { - groupLog.Errorf("leadership chair %q not found", orgMember) - } else { - checkedLeadership = append(checkedLeadership, orgMember) - } + if groupToValidate.Leadership == nil { + return + } + + var validChairs []*sigs.Chair + for _, orgMember := range groupToValidate.Leadership.Chairs { + if !kubevirtOrg.HasMember(orgMember.Github) { + groupLog.Errorf("leadership chair %q not found", orgMember) + } else { + validChairs = append(validChairs, orgMember) } - groupToValidate.Leadership.Chairs = checkedLeadership } + groupToValidate.Leadership.Chairs = validChairs } func validateSubprojects(groupToValidate *sigs.Group, groupLog *log.Entry, kubevirtOrg orgs.Org) { for _, subProject := range groupToValidate.SubProjects { subprojectLog := groupLog.WithField("subproject", subProject.Name) - foundOwners := validateOwnersReferences(subProject, subprojectLog) - subProject.Owners = foundOwners - - // check subproject leads - github handles are part of org - var checkedSubprojectChairs []*sigs.Lead - for _, orgMember := range subProject.Leads { - if !kubevirtOrg.HasMember(orgMember.Github) { - subprojectLog.Errorf("lead %q not found", orgMember) - } else { - checkedSubprojectChairs = append(checkedSubprojectChairs, orgMember) - } + subProject.Owners = validateOwnersReferences(subProject, subprojectLog) + subProject.Leads = validateLeads(subProject.Leads, kubevirtOrg, groupLog) + } +} + +func validateLeads(leadsToValidate []*sigs.Lead, kubevirtOrg orgs.Org, groupLog *log.Entry) []*sigs.Lead { + var validLeads []*sigs.Lead + for _, orgMember := range leadsToValidate { + if !kubevirtOrg.HasMember(orgMember.Github) { + groupLog.Errorf("lead %q not found", orgMember) + } else { + validLeads = append(validLeads, orgMember) } - subProject.Leads = checkedSubprojectChairs } + return validLeads } func validateOwnersReferences(subProject *sigs.SubProject, subprojectLog *log.Entry) []string { From faa0a73d7c34faf11c8d61a1397d79fa47f1ac1d Mon Sep 17 00:00:00 2001 From: Daniel Hiller Date: Thu, 12 Dec 2024 12:44:15 +0100 Subject: [PATCH 07/11] refactor,repo_groups: extract func Signed-off-by: Daniel Hiller --- generators/cmd/devstats/main.go | 39 +++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/generators/cmd/devstats/main.go b/generators/cmd/devstats/main.go index 78c8b121..439f6d03 100644 --- a/generators/cmd/devstats/main.go +++ b/generators/cmd/devstats/main.go @@ -72,6 +72,27 @@ func main() { log.Fatalf("failed to read sigs.yaml: %v", err) } + d := extractRepoGroups(sigsYAML) + + sql, err := generateRepoGroupsSQL(d) + if err != nil { + log.Fatal(fmt.Errorf("failed to generate sql: %w", err)) + } + + file, err := os.OpenFile(opts.outputPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0666) + if err != nil { + log.Fatal(fmt.Errorf("failed to write to file %q, %w", opts.outputPath, err)) + } + defer file.Close() + _, err = file.WriteString(sql) + if err != nil { + log.Fatal(fmt.Errorf("failed to write to file %q, %w", opts.outputPath, err)) + } + + log.Printf("output written to %q", opts.outputPath) +} + +func extractRepoGroups(sigsYAML *sigs.Sigs) RepoGroupsTemplateData { var d RepoGroupsTemplateData for _, sig := range sigsYAML.Sigs { repoGroup := RepoGroup{ @@ -102,23 +123,7 @@ func main() { repoGroup.Repos = repos d.RepoGroups = append(d.RepoGroups, repoGroup) } - - sql, err := generateRepoGroupsSQL(d) - if err != nil { - log.Fatal(fmt.Errorf("failed to generate sql: %w", err)) - } - - file, err := os.OpenFile(opts.outputPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0666) - if err != nil { - log.Fatal(fmt.Errorf("failed to write to file %q, %w", opts.outputPath, err)) - } - defer file.Close() - _, err = file.WriteString(sql) - if err != nil { - log.Fatal(fmt.Errorf("failed to write to file %q, %w", opts.outputPath, err)) - } - - log.Printf("output written to %q", opts.outputPath) + return d } //go:embed repo_groups.gosql From e1cb6594f2adba065ed589dcb10b4294abbf2068 Mon Sep 17 00:00:00 2001 From: Daniel Hiller Date: Fri, 13 Dec 2024 10:54:39 +0100 Subject: [PATCH 08/11] coverage: add make target Signed-off-by: Daniel Hiller --- Makefile | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index bf5675b8..dad0935b 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,21 @@ -.PHONY: generate validate-sigs install-metrics-binaries lint +.PHONY: make-artifacts-dir generate validate-sigs test coverage install-metrics-binaries lint export GOLANGCI_LINT_VERSION := v1.62.2 -ifndef $(GOPATH) +ifndef GOPATH GOPATH=$(shell go env GOPATH) export GOPATH endif +ifndef ARTIFACTS + ARTIFACTS=/tmp/artifacts + export ARTIFACTS +endif +ifndef COVERAGE_OUTPUT_PATH + COVERAGE_OUTPUT_PATH=${ARTIFACTS}/coverage.html + export COVERAGE_OUTPUT_PATH +endif + +make-artifacts-dir: + mkdir -p ${ARTIFACTS} generate: go run ./validators/cmd/sigs --dry-run=false @@ -17,6 +28,11 @@ validate-sigs: test: go test ./... +coverage: make-artifacts-dir + if ! command -V covreport; then go install github.com/cancue/covreport@latest; fi + go test ./... -coverprofile=/tmp/coverage.out + covreport -i /tmp/coverage.out -o ${COVERAGE_OUTPUT_PATH} + install-metrics-binaries: if ! command -V golangci-lint; then curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b ${GOPATH}/bin ${GOLANGCI_LINT_VERSION} ; fi From 06e39c80c4cdf9c312c92e071ba4d41528a1d074 Mon Sep 17 00:00:00 2001 From: Daniel Hiller Date: Thu, 16 Jan 2025 16:27:28 +0100 Subject: [PATCH 09/11] review: fix issues Signed-off-by: Daniel Hiller --- .golangci.yaml | 2 +- Makefile | 26 +++++++++++----------- validators/cmd/sigs/testdata/existing-file | 2 +- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 6b266392..3f2196cf 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -3,4 +3,4 @@ linters: - gocyclo linters-settings: gocyclo: - min-complexity: 10 \ No newline at end of file + min-complexity: 10 diff --git a/Makefile b/Makefile index dad0935b..a67d7804 100644 --- a/Makefile +++ b/Makefile @@ -1,21 +1,23 @@ -.PHONY: make-artifacts-dir generate validate-sigs test coverage install-metrics-binaries lint +.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 - ARTIFACTS=/tmp/artifacts - export ARTIFACTS + export ARTIFACTS := $(WORKDIR)/artifacts endif ifndef COVERAGE_OUTPUT_PATH - COVERAGE_OUTPUT_PATH=${ARTIFACTS}/coverage.html - export COVERAGE_OUTPUT_PATH + COVERAGE_OUTPUT_PATH := ${ARTIFACTS}/coverage.html endif -make-artifacts-dir: +work-dirs: mkdir -p ${ARTIFACTS} + mkdir -p ${LOCAL_BIN} generate: go run ./validators/cmd/sigs --dry-run=false @@ -28,13 +30,11 @@ validate-sigs: test: go test ./... -coverage: make-artifacts-dir - if ! command -V covreport; then go install github.com/cancue/covreport@latest; fi +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} + covreport -i /tmp/coverage.out -o $(COVERAGE_OUTPUT_PATH) -install-metrics-binaries: - if ! command -V golangci-lint; then curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b ${GOPATH}/bin ${GOLANGCI_LINT_VERSION} ; fi - -lint: install-metrics-binaries +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 diff --git a/validators/cmd/sigs/testdata/existing-file b/validators/cmd/sigs/testdata/existing-file index 89626620..982793c3 100644 --- a/validators/cmd/sigs/testdata/existing-file +++ b/validators/cmd/sigs/testdata/existing-file @@ -1 +1 @@ -whatever \ No newline at end of file +whatever From 5675627872a71c57f43819836c89b94da0382f12 Mon Sep 17 00:00:00 2001 From: Daniel Hiller Date: Thu, 16 Jan 2025 17:40:39 +0100 Subject: [PATCH 10/11] chore: fix linting errors Signed-off-by: Daniel Hiller --- generators/cmd/contributions/main.go | 229 +++++++++++++++---------- generators/cmd/contributions/report.go | 12 +- pkg/contributions/user-contribution.go | 7 + 3 files changed, 150 insertions(+), 98 deletions(-) diff --git a/generators/cmd/contributions/main.go b/generators/cmd/contributions/main.go index a3a74866..5074ea3a 100644 --- a/generators/cmd/contributions/main.go +++ b/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,134 @@ 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) + communityReportGen := newCommunityReportGenerator(contributionReportOpts, generator) + communityReportGen.determineReporterAndUserNames() + communityReportGen.generateReportPerUser() + communityReportGen.printReportSummary() + communityReportGen.handleReportOutput() +} + +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) determineReporterAndUserNames() communityReportGenerator { + reporter := NewDefaultReporter(g.contributionReportOpts, defaultConfig) + userNames := []string{g.contributionReportOpts.Username} + if g.contributionReportOpts.Username != "" { + return communityReportGenerator{reporter: reporter, userNames: userNames} + } + + if !g.contributionReportOpts.ReportAll { + 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) + userNames = ownersYAML.AllReviewers() + userNames = append(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 } + userNames = ownersAliases.Resolve(userNames) + userNames = uniq(userNames) + sort.Strings(userNames) + } else if g.contributionReportOpts.OrgsConfigFilePath != "" { + orgsYAML, err := orgs.ReadFile(g.contributionReportOpts.OrgsConfigFilePath) + if err != nil { + log.Fatalf("invalid arguments: %v", err) + } + userNames = orgsYAML.Orgs[g.contributionReportOpts.Org].Members + } + + return communityReportGenerator{reporter: reporter, userNames: userNames} +} + +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 -} diff --git a/generators/cmd/contributions/report.go b/generators/cmd/contributions/report.go index fb5bd57e..01b02034 100644 --- a/generators/cmd/contributions/report.go +++ b/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 diff --git a/pkg/contributions/user-contribution.go b/pkg/contributions/user-contribution.go index 70f2318c..4d46af27 100644 --- a/pkg/contributions/user-contribution.go +++ b/pkg/contributions/user-contribution.go @@ -39,6 +39,10 @@ type ContributionReportGenerator struct { } func NewContributionReportGenerator(opts ContributionReportGeneratorOptions) (*ContributionReportGenerator, error) { + err := opts.validate() + if err != nil { + return nil, fmt.Errorf("validation failed: %v", err) + } token, err := os.ReadFile(opts.GithubTokenPath) if err != nil { return nil, fmt.Errorf("failed to use github token path %s: %v", opts.GithubTokenPath, err) @@ -261,6 +265,9 @@ func getUserId(client *githubv4.Client, username string) (string, error) { func writeActivityToFile(yamlObject interface{}, dir, fileName string) error { tempFile, err := os.CreateTemp(dir, fileName) + if err != nil { + return err + } defer tempFile.Close() encoder := yaml.NewEncoder(tempFile) err = encoder.Encode(&yamlObject) From 141960068f543b668ad926e1f33b4d104189ffba Mon Sep 17 00:00:00 2001 From: Daniel Hiller Date: Thu, 16 Jan 2025 17:45:43 +0100 Subject: [PATCH 11/11] chore,refactoring: community report Signed-off-by: Daniel Hiller --- generators/cmd/contributions/main.go | 37 ++++++++++++++-------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/generators/cmd/contributions/main.go b/generators/cmd/contributions/main.go index 5074ea3a..bdeef16f 100644 --- a/generators/cmd/contributions/main.go +++ b/generators/cmd/contributions/main.go @@ -158,11 +158,7 @@ func main() { log.Fatalf("failed to create report generator: %v", err) } - communityReportGen := newCommunityReportGenerator(contributionReportOpts, generator) - communityReportGen.determineReporterAndUserNames() - communityReportGen.generateReportPerUser() - communityReportGen.printReportSummary() - communityReportGen.handleReportOutput() + newCommunityReportGenerator(contributionReportOpts, generator).generateRequestedCommunityReport() } func newCommunityReportGenerator(contributionReportOpts *contributionReportOptions, generator *contributions.ContributionReportGenerator) *communityReportGenerator { @@ -180,15 +176,22 @@ type communityReportGenerator struct { userNames []string } -func (g *communityReportGenerator) determineReporterAndUserNames() communityReportGenerator { - reporter := NewDefaultReporter(g.contributionReportOpts, defaultConfig) - userNames := []string{g.contributionReportOpts.Username} +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 communityReportGenerator{reporter: reporter, userNames: userNames} + return } if !g.contributionReportOpts.ReportAll { - reporter = NewInactiveOnlyReporter(g.contributionReportOpts, defaultConfig) + g.reporter = NewInactiveOnlyReporter(g.contributionReportOpts, defaultConfig) } if g.contributionReportOpts.OwnersFilePath != "" { @@ -196,8 +199,8 @@ func (g *communityReportGenerator) determineReporterAndUserNames() communityRepo if err != nil { log.Fatalf("invalid arguments: %v", err) } - userNames = ownersYAML.AllReviewers() - userNames = append(userNames, ownersYAML.AllApprovers()...) + g.userNames = ownersYAML.AllReviewers() + g.userNames = append(g.userNames, ownersYAML.AllApprovers()...) ownersAliasesPath := g.contributionReportOpts.ownersAliasesFilePath() stat, err := os.Stat(ownersAliasesPath) @@ -208,18 +211,16 @@ func (g *communityReportGenerator) determineReporterAndUserNames() communityRepo log.Fatalf("invalid aliases file %q: %v", ownersAliasesPath, err) } } - userNames = ownersAliases.Resolve(userNames) - userNames = uniq(userNames) - sort.Strings(userNames) + 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) } - userNames = orgsYAML.Orgs[g.contributionReportOpts.Org].Members + g.userNames = orgsYAML.Orgs[g.contributionReportOpts.Org].Members } - - return communityReportGenerator{reporter: reporter, userNames: userNames} } func uniq(elements ...[]string) []string {