Skip to content

Commit

Permalink
more cleanups
Browse files Browse the repository at this point in the history
  • Loading branch information
xrstf committed May 29, 2024
1 parent 9ad4158 commit b1c7c4d
Show file tree
Hide file tree
Showing 10 changed files with 290 additions and 85 deletions.
6 changes: 3 additions & 3 deletions hack/generate-client.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ package main
import (
"bytes"
"go/format"
"io/ioutil"
"log"
"os"
"path/filepath"
"strings"
"text/template"
Expand Down Expand Up @@ -53,7 +53,7 @@ func main() {
for _, templateFile := range templates {
log.Printf("Rendering %s...", templateFile)

content, err := ioutil.ReadFile(templateFile)
content, err := os.ReadFile(templateFile)
if err != nil {
log.Fatalf("Failed to read %s -- did you run this from the root directory?: %v", templateFile, err)
}
Expand All @@ -70,7 +70,7 @@ func main() {

filename := filepath.Join("pkg/github", strings.TrimSuffix(filepath.Base(templateFile), ".tmpl"))

err = ioutil.WriteFile(filename, source, 0644)
err = os.WriteFile(filename, source, 0644)
if err != nil {
log.Fatalf("Failed to write %s: %v", filename, err)
}
Expand Down
52 changes: 51 additions & 1 deletion pkg/changelog/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ package changelog

import (
"fmt"
"slices"
"strings"

"k8c.io/gchl/pkg/types"

Expand Down Expand Up @@ -82,15 +84,63 @@ func (g *Generator) groupChanges(changes []Change) []ChangeGroup {
// List() sorts for us
changeTypes := sets.List(sets.KeySet(tempMap))

// change groups are supposed to be sorted alphabetically, except for some
// well known groups which we want to put in specific spots
//
// 1. New Features
// 2. API Changes
// 3. Deprecations
// ...
// N-2. Miscellaneous
// N-1. Chore
// N. Updates
//
// Everything between 3. and N-2 is sorted alphabetically.
changeTypes = sortSemantically(changeTypes)

result := []ChangeGroup{}
for _, changeType := range changeTypes {
changes := tempMap[changeType]

result = append(result, ChangeGroup{
Title: changeType.Title(),
Type: changeType,
Changes: changes,
})
}

return result
}

func sortSemantically(changeTypes []ChangeType) []ChangeType {
slices.SortStableFunc(changeTypes, func(a, b ChangeType) int {
for _, check := range []ChangeType{
ChangeTypeFeature,
ChangeTypeAPIChange,
ChangeTypeDeprecation,
} {
if a == check {
return -1
}
if b == check {
return 1
}
}

for _, check := range []ChangeType{
ChangeTypeUpdate,
ChangeTypeChore,
ChangeTypeMisc,
} {
if a == check {
return 1
}
if b == check {
return -1
}
}

return strings.Compare(string(a), string(b))
})

return changeTypes
}
32 changes: 1 addition & 31 deletions pkg/changelog/generator_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2022 The Kubermatic Kubernetes Platform contributors.
Copyright 2024 The Kubermatic Kubernetes Platform contributors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand All @@ -26,36 +26,6 @@ import (
"gopkg.in/yaml.v3"
)

// func TestHumanReadableChangeType(t *testing.T) {
// testcases := []struct {
// identifier string
// expected string
// }{
// {
// identifier: "breaking change",
// expected: "Breaking Change",
// },
// {
// identifier: "api change",
// expected: "API Change",
// },
// {
// identifier: "api-change",
// expected: "API Changes",
// },
// }

// for _, testcase := range testcases {
// t.Run(testcase.identifier, func(t *testing.T) {
// result := humanReadableChangeType(testcase.identifier)

// if result != testcase.expected {
// t.Fatalf("Expected %q, got %q.", testcase.expected, result)
// }
// })
// }
// }

type generateChangesTestcase struct {
PR types.PullRequest `yaml:"pr"`
Changes []Change `yaml:"changes"`
Expand Down
42 changes: 27 additions & 15 deletions pkg/changelog/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,23 +143,22 @@ func splitIntoLines(text string) []string {
}

func itemToChange(explicitType ChangeType, breaking bool, text string) Change {
text = strings.TrimSuffix(text, ".")
text = inflect.Capitalize(text)
text = harmonizeLinePrefixes(text)

// item is breaking if it's in its text or the entire release-note block
// is marked as breaking
breaking = breaking || isBreakingChange(text)

if explicitType == "" {
switch {
case isBugfix(text):
explicitType = ChangeTypeBugfix
case isUpdate(text):
explicitType = ChangeTypeUpdate
default:
explicitType = ChangeTypeMisc
}
text = strings.TrimSuffix(text, ".")
text = removeActionRequired(text)
text = inflect.Capitalize(text)
text = harmonizeLinePrefixes(text)

switch {
case isBugfix(text):
explicitType = ChangeTypeBugfix
case isUpdate(text):
explicitType = ChangeTypeUpdate
case explicitType == "":
explicitType = ChangeTypeMisc
}

return Change{
Expand All @@ -175,15 +174,27 @@ func isBreakingChange(text string) bool {
}

func isBugfix(releaseNote string) bool {
return strings.HasPrefix(releaseNote, "Fix ")
return strings.HasPrefix(strings.ToLower(releaseNote), "fix ")
}

var isUpdateRegex = regexp.MustCompile(`updat(es|ed|ing|e) (.+)( version)? to (.+?)$`)
var isUpdateRegex = regexp.MustCompile(`^updat(es|ed|ing|e) `)

func isUpdate(releaseNote string) bool {
return isUpdateRegex.MatchString(strings.ToLower(releaseNote))
}

var actionRequiredRegex = regexp.MustCompile(`(?i)^[[*]*action required[\]*:]*`)

// Remove redundant "ACTION REQUIRED" prefixes: breaking changes are already
// grouped into a dedicated section in the changelog, no need to prefix every
// item with the same yelling.
func removeActionRequired(text string) string {
text = actionRequiredRegex.ReplaceAllLiteralString(text, "")
text = strings.TrimSpace(text)

return text
}

func harmonizeLinePrefixes(text string) string {
replacements := map[string]string{
"Fixes": "Fix",
Expand Down Expand Up @@ -212,6 +223,7 @@ func harmonizeLinePrefixes(text string) string {
"Removes": "Remove",
"Removed": "Remove",
"Removing": "Remove",
"Resolved": "Resolve",
"Deprecates": "Deprecate",
"Deprecated": "Deprecate",
"Deprecating": "Deprecate",
Expand Down
105 changes: 105 additions & 0 deletions pkg/changelog/parse_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
Copyright 2024 The Kubermatic Kubernetes Platform contributors.
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.
*/

package changelog

import (
"testing"
)

func TestIsUpdate(t *testing.T) {
testcases := []struct {
text string
expected bool
}{
{
text: "Update OSM to v1.5.2; fixing cloud-init bootstrapping issues on Ubuntu 22.04 on Azure",
expected: true,
},
{
text: "updates operating-system-manager to v1.5.1.",
expected: true,
},
{
text: "update Metering to v1.2.1.",
expected: true,
},
{
text: "Updated to Go 1.22.2",
expected: true,
},
{
text: "Update Cilium to 1.14.9 and 1.13.14",
expected: true,
},
{
text: "updating to Kubernetes 1.29",
expected: true,
},
{
text: "Update Vertical Pod Autoscaler to 1.0",
expected: true,
},
}

for _, testcase := range testcases {
t.Run(testcase.text, func(t *testing.T) {
result := isUpdate(testcase.text)

if result != testcase.expected {
t.Fatalf("Expected %v, got %v.", testcase.expected, result)
}
})
}
}

func TestRemoveActionRequired(t *testing.T) {
testcases := []struct {
input string
expected string
}{
{
input: "Action required: if you use `velero.restic.deploy: true`...",
expected: "if you use `velero.restic.deploy: true`...",
},
{
input: "**ACTION REQUIRED**: For velero helm chart upgrade. If running...",
expected: "For velero helm chart upgrade. If running...",
},
{
input: "Action required: [User-mla] If you had copied `values.yaml...",
expected: "[User-mla] If you had copied `values.yaml...",
},
{
input: "[ACTION REQUIRED] KubeLB: The prefix for the tenant namespaces created...",
expected: "KubeLB: The prefix for the tenant namespaces created...",
},
{
input: "[Action Required] The field `ovdcNetwork` in `cluster` and `preset...",
expected: "The field `ovdcNetwork` in `cluster` and `preset...",
},
}

for _, testcase := range testcases {
t.Run(testcase.input, func(t *testing.T) {
result := removeActionRequired(testcase.input)

if result != testcase.expected {
t.Fatalf("Expected %q, got %q.", testcase.expected, result)
}
})
}
}
19 changes: 19 additions & 0 deletions pkg/changelog/testdata/multi-typed-release-note.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
pr:
labels:
- kind/bug

body: |
hello world
asdasd
```release-note
* Update operating-system-manager to v1.5.1.
* [ACTION REQUIRED] The latest Ubuntu 22.04 images ship with cloud-init 24.x package. This package has breaking changes and thus rendered our OSPs as incompatible. It's recommended to refresh your machines with latest provided OSPs to ensure that a system-wide package update, that updates cloud-init to 24.x, doesn't break the machines.
```
changes:
- releaseNote: "The latest Ubuntu 22.04 images ship with cloud-init 24.x package. This package has breaking changes and thus rendered our OSPs as incompatible. It's recommended to refresh your machines with latest provided OSPs to ensure that a system-wide package update, that updates cloud-init to 24.x, doesn't break the machines"
breaking: true
type: bugfix
- releaseNote: "Update operating-system-manager to v1.5.1"
type: update
Loading

0 comments on commit b1c7c4d

Please sign in to comment.