From d297dcf1aa72180aff18f870a8d071719f83c5e8 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Wed, 1 Apr 2020 16:53:58 -0400 Subject: [PATCH 01/21] WIP: templates, examples, and prototype code for autogenerating CHANGELOG and migration guide (#2761) --- changelog/fragments/00-template.yaml | 27 ++++ changelog/fragments/example.yaml | 25 ++++ changelog/generate.go | 210 +++++++++++++++++++++++++++ 3 files changed, 262 insertions(+) create mode 100644 changelog/fragments/00-template.yaml create mode 100644 changelog/fragments/example.yaml create mode 100644 changelog/generate.go diff --git a/changelog/fragments/00-template.yaml b/changelog/fragments/00-template.yaml new file mode 100644 index 00000000000..e0eef1c428a --- /dev/null +++ b/changelog/fragments/00-template.yaml @@ -0,0 +1,27 @@ +# entries is a list of entries to include in +# release notes and/or the migration guide +entries: + - description: > + Description is the line that shows up in the CHANGELOG. This + should be formatted as markdown and be on a single line. + + # kind is one of: + # - addition + # - change + # - deprecation + # - removal + # - bugfix + kind: "" + + # Is this a breaking change? + breaking: false + + # What is the pull request number (without the "#")? + pull_request: 0 + + # Migration can be defined to automatically add a section to + # the migration guide. This is required for breaking changes. + migration: + title: Header text for the migration section + body: > + Body of the migration section. \ No newline at end of file diff --git a/changelog/fragments/example.yaml b/changelog/fragments/example.yaml new file mode 100644 index 00000000000..8d227498cfa --- /dev/null +++ b/changelog/fragments/example.yaml @@ -0,0 +1,25 @@ +entries: + - description: > + This is the description for a change that was made. + It's a great change! + kind: change + breaking: false + pull_request: 3001 + + - description: > + Here is another change from the same PR. Using '>' means + I can span multiple lines in YAML and the parser sees it as + just one line. + kind: change + breaking: true + pull_request: 3001 + migration: + title: Breaking changes must have migrations + body: > + When auto-generating the migration guide, we expect a blurb + about every single breaking change. + + - description: Here's a bug we fixed! + kind: bugfix + breaking: false + pull_request: 3003 \ No newline at end of file diff --git a/changelog/generate.go b/changelog/generate.go new file mode 100644 index 00000000000..f25ac6754f2 --- /dev/null +++ b/changelog/generate.go @@ -0,0 +1,210 @@ +package main + +import ( + "errors" + "flag" + "fmt" + "io/ioutil" + "os" + "path/filepath" + "strings" + + "gopkg.in/yaml.v2" + + log "github.com/sirupsen/logrus" +) + +func main() { + var ( + title string + fragmentsDir string + changelogFile string + ) + + flag.StringVar(&title, "title", "", "Title for generated CHANGELOG and migration guide sections") + flag.StringVar(&fragmentsDir, "fragments-dir", filepath.Join("changelog", "fragments"), "Path to changelog fragments directory") + flag.StringVar(&changelogFile, "changelog", "CHANGELOG.md", "Path to CHANGELOG.md") + flag.Parse() + + if title == "" { + log.Fatalf("flag '-title' is required!") + } + files, err := ioutil.ReadDir(fragmentsDir) + if err != nil { + log.Fatalf("failed to read fragments directory: %v", err) + } + + changelog := map[EntryKind][]string{} + haveEntries := false + + for _, fragFile := range files { + if fragFile.Name() == "00-template.yaml" { + continue + } + if fragFile.IsDir() { + log.Warnf("Skipping directory %q", fragFile.Name()) + continue + } + if filepath.Ext(fragFile.Name()) != ".yaml" || fragFile.IsDir() { + log.Warnf("Skipping non-YAML file %q", fragFile.Name()) + continue + } + path := filepath.Join(fragmentsDir, fragFile.Name()) + f, err := os.Open(path) + if err != nil { + log.Fatalf("Failed to open fragment file %q: %v", fragFile.Name(), err) + } + + decoder := yaml.NewDecoder(f) + fragment := &Fragment{} + if err := decoder.Decode(fragment); err != nil { + log.Fatalf("Failed to parse fragment file %q: %v", fragFile.Name(), err) + } + + if err := fragment.Validate(); err != nil { + log.Fatalf("Failed to validate fragment file %q: %v", fragFile.Name(), err) + } + + for _, e := range fragment.Entries { + changelog[e.Kind] = append(changelog[e.Kind], e.ToChangelogString()) + haveEntries = true + } + } + + if !haveEntries { + log.Fatal("No new CHANGELOG entries found!") + } + + var sb strings.Builder + order := []EntryKind{ + Addition, + Change, + Removal, + Deprecation, + Bugfix, + } + sb.WriteString(fmt.Sprintf("## %s\n\n", title)) + for _, k := range order { + if entries, ok := changelog[k]; ok { + sb.WriteString(k.ToHeader() + "\n\n") + for _, e := range entries { + sb.WriteString(e + "\n") + } + sb.WriteString("\n") + } + } + + existingFile, err := ioutil.ReadFile(changelogFile) + if err != nil { + log.Infof("No existing CHANGELOG file to prepend to") + } + sb.Write(existingFile) + fmt.Print(sb.String()) +} + +type Fragment struct { + Entries []Entry `yaml:"entries"` +} + +type Entry struct { + Description string `yaml:"description"` + Kind EntryKind `yaml:"kind"` + Breaking bool `yaml:"breaking"` + Migration *Migration `yaml:"migration,omitempty"` + PullRequest *uint `yaml:"pull_request,omitempty"` +} + +type EntryKind string + +const ( + Addition EntryKind = "addition" + Change EntryKind = "change" + Removal EntryKind = "removal" + Deprecation EntryKind = "deprecation" + Bugfix EntryKind = "bugfix" +) + +func (k EntryKind) ToHeader() string { + switch k { + case Addition: + return "### Additions" + case Change: + return "### Changes" + case Removal: + return "### Removals" + case Deprecation: + return "### Deprecations" + case Bugfix: + return "### Bug Fixes" + default: + panic("invalid entry kind; NOTE TO DEVELOPERS: check EntryKind.Validate") + } +} + +type Migration struct { + Title string `yaml:"title"` + Body string `yaml:"body"` +} + +func (f *Fragment) Validate() error { + for i, e := range f.Entries { + if err := e.Validate(); err != nil { + return fmt.Errorf("entry[%d] invalid: %v", i, err) + } + } + return nil +} + +func (e *Entry) Validate() error { + if err := e.Kind.Validate(); err != nil { + return fmt.Errorf("invalid kind: %v", err) + } + + if e.Breaking && e.Kind != Change && e.Kind != Removal { + return fmt.Errorf("breaking changes can only be kind %q or %q, got %q", Change, Removal, e.Kind) + } + + if e.Breaking && e.Migration == nil { + return fmt.Errorf("breaking changes require migration descriptions") + } + + if e.Migration != nil { + if err := e.Migration.Validate(); err != nil { + return fmt.Errorf("invalid migration: %v", err) + } + } + return nil +} + +func (e Entry) ToChangelogString() string { + text := strings.Trim(e.Description, "\n") + if e.Breaking { + text = fmt.Sprintf("**Breaking Change**: %s", text) + } + if !strings.HasSuffix(text, ".") && !strings.HasSuffix(text, "!") { + text = fmt.Sprintf("%s.", text) + } + if e.PullRequest != nil { + text = fmt.Sprintf("%s ([#%d](https://github.com/operator-framework/operator-sdk/pull/%d))", text, *e.PullRequest, *e.PullRequest) + } + return fmt.Sprintf("- %s", text) +} + +func (k EntryKind) Validate() error { + for _, t := range []EntryKind{Addition, Change, Removal, Deprecation, Bugfix} { + if k == t { + return nil + } + } + return fmt.Errorf("%q is not a supported kind", k) +} + +func (m Migration) Validate() error { + if len(m.Title) == 0 { + return errors.New("title not specified") + } + if len(m.Body) == 0 { + return errors.New("body not specified") + } + return nil +} From fc65972d5e5acba5483fb678847f85fc444f8794 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Thu, 2 Apr 2020 11:55:11 -0400 Subject: [PATCH 02/21] change migration title to header, split things out into functions, generate migration guide update --- changelog/fragments/00-template.yaml | 2 +- changelog/fragments/example.yaml | 2 +- changelog/generate.go | 141 +++++++++++++++++++++------ 3 files changed, 111 insertions(+), 34 deletions(-) diff --git a/changelog/fragments/00-template.yaml b/changelog/fragments/00-template.yaml index e0eef1c428a..125cccda652 100644 --- a/changelog/fragments/00-template.yaml +++ b/changelog/fragments/00-template.yaml @@ -22,6 +22,6 @@ entries: # Migration can be defined to automatically add a section to # the migration guide. This is required for breaking changes. migration: - title: Header text for the migration section + header: Header text for the migration section body: > Body of the migration section. \ No newline at end of file diff --git a/changelog/fragments/example.yaml b/changelog/fragments/example.yaml index 8d227498cfa..a6161503826 100644 --- a/changelog/fragments/example.yaml +++ b/changelog/fragments/example.yaml @@ -14,7 +14,7 @@ entries: breaking: true pull_request: 3001 migration: - title: Breaking changes must have migrations + header: Breaking changes must have migrations body: > When auto-generating the migration guide, we expect a blurb about every single breaking change. diff --git a/changelog/generate.go b/changelog/generate.go index f25ac6754f2..1563084d2df 100644 --- a/changelog/generate.go +++ b/changelog/generate.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "errors" "flag" "fmt" @@ -19,24 +20,56 @@ func main() { title string fragmentsDir string changelogFile string + migrationFile string ) - flag.StringVar(&title, "title", "", "Title for generated CHANGELOG and migration guide sections") - flag.StringVar(&fragmentsDir, "fragments-dir", filepath.Join("changelog", "fragments"), "Path to changelog fragments directory") - flag.StringVar(&changelogFile, "changelog", "CHANGELOG.md", "Path to CHANGELOG.md") + flag.StringVar(&title, "title", "", + "Title for generated CHANGELOG and migration guide sections") + flag.StringVar(&fragmentsDir, "fragments-dir", filepath.Join("changelog", "fragments"), + "Path to changelog fragments directory") + flag.StringVar(&changelogFile, "changelog", "CHANGELOG.md", + "Path to CHANGELOG") + flag.StringVar(&migrationFile, "migration-guide", + filepath.Join("website", "content", "en", "docs", "migration", "version-upgrade-guide.md"), + "Path to migration guide") flag.Parse() if title == "" { log.Fatalf("flag '-title' is required!") } - files, err := ioutil.ReadDir(fragmentsDir) + + entries, err := LoadEntries(fragmentsDir) if err != nil { - log.Fatalf("failed to read fragments directory: %v", err) + log.Fatalf("failed to load fragments: %v", err) + } + if len(entries) == 0 { + log.Fatalf("no entries found") } - changelog := map[EntryKind][]string{} - haveEntries := false + if err := UpdateChangelog(Config{ + File: changelogFile, + Title: title, + Entries: entries, + }); err != nil { + log.Fatalf("failed to update CHANGELOG: %v", err) + } + if err := UpdateMigrationGuide(Config{ + File: migrationFile, + Title: title, + Entries: entries, + }); err != nil { + log.Fatalf("failed to update migration guide: %v", err) + } +} + +func LoadEntries(fragmentsDir string) ([]Entry, error) { + files, err := ioutil.ReadDir(fragmentsDir) + if err != nil { + return nil, fmt.Errorf("failed to read fragments directory: %w", err) + } + + var entries []Entry for _, fragFile := range files { if fragFile.Name() == "00-template.yaml" { continue @@ -52,30 +85,31 @@ func main() { path := filepath.Join(fragmentsDir, fragFile.Name()) f, err := os.Open(path) if err != nil { - log.Fatalf("Failed to open fragment file %q: %v", fragFile.Name(), err) + return nil, fmt.Errorf("failed to open fragment file %q: %w", fragFile.Name(), err) } decoder := yaml.NewDecoder(f) - fragment := &Fragment{} - if err := decoder.Decode(fragment); err != nil { - log.Fatalf("Failed to parse fragment file %q: %v", fragFile.Name(), err) + fragment := Fragment{} + if err := decoder.Decode(&fragment); err != nil { + return nil, fmt.Errorf("failed to parse fragment file %q: %w", fragFile.Name(), err) } if err := fragment.Validate(); err != nil { - log.Fatalf("Failed to validate fragment file %q: %v", fragFile.Name(), err) + return nil, fmt.Errorf("failed to validate fragment file %q: %w", fragFile.Name(), err) } - for _, e := range fragment.Entries { - changelog[e.Kind] = append(changelog[e.Kind], e.ToChangelogString()) - haveEntries = true - } + entries = append(entries, fragment.Entries...) } + return entries, nil +} - if !haveEntries { - log.Fatal("No new CHANGELOG entries found!") +func UpdateChangelog(c Config) error { + changelog := map[EntryKind][]string{} + for _, e := range c.Entries { + changelog[e.Kind] = append(changelog[e.Kind], e.ToChangelogString()) } - var sb strings.Builder + var bb bytes.Buffer order := []EntryKind{ Addition, Change, @@ -83,23 +117,55 @@ func main() { Deprecation, Bugfix, } - sb.WriteString(fmt.Sprintf("## %s\n\n", title)) + bb.WriteString(fmt.Sprintf("## %s\n\n", c.Title)) for _, k := range order { if entries, ok := changelog[k]; ok { - sb.WriteString(k.ToHeader() + "\n\n") + bb.WriteString(k.ToHeader() + "\n\n") for _, e := range entries { - sb.WriteString(e + "\n") + bb.WriteString(e + "\n") } - sb.WriteString("\n") + bb.WriteString("\n") } } - existingFile, err := ioutil.ReadFile(changelogFile) + existingFile, err := ioutil.ReadFile(c.File) if err != nil { - log.Infof("No existing CHANGELOG file to prepend to") + return fmt.Errorf("could not read CHANGELOG: %v", err) + } + bb.Write(existingFile) + + if err := ioutil.WriteFile(c.File, bb.Bytes(), 0644); err != nil { + return fmt.Errorf("could not write CHANGELOG file: %v", err) } - sb.Write(existingFile) - fmt.Print(sb.String()) + return nil +} + +func UpdateMigrationGuide(c Config) error { + var bb bytes.Buffer + existingFile, err := ioutil.ReadFile(c.File) + if err != nil { + return fmt.Errorf("could not read migration guide: %v", err) + } + bb.Write(bytes.Trim(existingFile, "\n")) + + bb.WriteString(fmt.Sprintf("\n\n## %s\n\n", c.Title)) + haveMigrations := false + for _, e := range c.Entries { + if e.Migration != nil { + haveMigrations = true + bb.WriteString(fmt.Sprintf("### %s\n\n", e.Migration.Header)) + bb.WriteString(fmt.Sprintf("%s\n\n", strings.Trim(e.Migration.Body, "\n"))) + bb.WriteString(fmt.Sprintf("See %s for more details.\n\n", e.PullRequestLink())) + } + } + if !haveMigrations { + bb.WriteString("There are no migrations for this release! :tada:\n\n") + } + + if err := ioutil.WriteFile(c.File, bytes.TrimSuffix(bb.Bytes(), []byte("\n")), 0644); err != nil { + return fmt.Errorf("could not write migration guide: %v", err) + } + return nil } type Fragment struct { @@ -142,8 +208,14 @@ func (k EntryKind) ToHeader() string { } type Migration struct { - Title string `yaml:"title"` - Body string `yaml:"body"` + Header string `yaml:"header"` + Body string `yaml:"body"` +} + +type Config struct { + File string + Title string + Entries []Entry } func (f *Fragment) Validate() error { @@ -185,11 +257,16 @@ func (e Entry) ToChangelogString() string { text = fmt.Sprintf("%s.", text) } if e.PullRequest != nil { - text = fmt.Sprintf("%s ([#%d](https://github.com/operator-framework/operator-sdk/pull/%d))", text, *e.PullRequest, *e.PullRequest) + text = fmt.Sprintf("%s (%s)", text, e.PullRequestLink()) } return fmt.Sprintf("- %s", text) } +func (e Entry) PullRequestLink() string { + const repo = "github.com/operator-framework/operator-sdk" + return fmt.Sprintf("[#%d](https://%s/pull/%d)", *e.PullRequest, repo, *e.PullRequest) +} + func (k EntryKind) Validate() error { for _, t := range []EntryKind{Addition, Change, Removal, Deprecation, Bugfix} { if k == t { @@ -200,8 +277,8 @@ func (k EntryKind) Validate() error { } func (m Migration) Validate() error { - if len(m.Title) == 0 { - return errors.New("title not specified") + if len(m.Header) == 0 { + return errors.New("header not specified") } if len(m.Body) == 0 { return errors.New("body not specified") From d033d20bb7022779d42a030b7a782cf5015d392e Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Thu, 2 Apr 2020 16:00:52 -0400 Subject: [PATCH 03/21] unexport types and functions --- changelog/generate.go | 110 +++++++++++++++++++++--------------------- 1 file changed, 55 insertions(+), 55 deletions(-) diff --git a/changelog/generate.go b/changelog/generate.go index 1563084d2df..5e35a953a0e 100644 --- a/changelog/generate.go +++ b/changelog/generate.go @@ -38,15 +38,15 @@ func main() { log.Fatalf("flag '-title' is required!") } - entries, err := LoadEntries(fragmentsDir) + entries, err := loadEntries(fragmentsDir) if err != nil { log.Fatalf("failed to load fragments: %v", err) } if len(entries) == 0 { - log.Fatalf("no entries found") + log.Fatalf("no Entries found") } - if err := UpdateChangelog(Config{ + if err := updateChangelog(config{ File: changelogFile, Title: title, Entries: entries, @@ -54,7 +54,7 @@ func main() { log.Fatalf("failed to update CHANGELOG: %v", err) } - if err := UpdateMigrationGuide(Config{ + if err := updateMigrationGuide(config{ File: migrationFile, Title: title, Entries: entries, @@ -63,13 +63,13 @@ func main() { } } -func LoadEntries(fragmentsDir string) ([]Entry, error) { +func loadEntries(fragmentsDir string) ([]entry, error) { files, err := ioutil.ReadDir(fragmentsDir) if err != nil { return nil, fmt.Errorf("failed to read fragments directory: %w", err) } - var entries []Entry + var entries []entry for _, fragFile := range files { if fragFile.Name() == "00-template.yaml" { continue @@ -89,12 +89,12 @@ func LoadEntries(fragmentsDir string) ([]Entry, error) { } decoder := yaml.NewDecoder(f) - fragment := Fragment{} + fragment := fragment{} if err := decoder.Decode(&fragment); err != nil { return nil, fmt.Errorf("failed to parse fragment file %q: %w", fragFile.Name(), err) } - if err := fragment.Validate(); err != nil { + if err := fragment.validate(); err != nil { return nil, fmt.Errorf("failed to validate fragment file %q: %w", fragFile.Name(), err) } @@ -103,24 +103,24 @@ func LoadEntries(fragmentsDir string) ([]Entry, error) { return entries, nil } -func UpdateChangelog(c Config) error { - changelog := map[EntryKind][]string{} +func updateChangelog(c config) error { + changelog := map[entryKind][]string{} for _, e := range c.Entries { - changelog[e.Kind] = append(changelog[e.Kind], e.ToChangelogString()) + changelog[e.Kind] = append(changelog[e.Kind], e.toChangelogString()) } var bb bytes.Buffer - order := []EntryKind{ - Addition, - Change, - Removal, - Deprecation, - Bugfix, + order := []entryKind{ + addition, + change, + removal, + deprecation, + bugfix, } bb.WriteString(fmt.Sprintf("## %s\n\n", c.Title)) for _, k := range order { if entries, ok := changelog[k]; ok { - bb.WriteString(k.ToHeader() + "\n\n") + bb.WriteString(k.toHeader() + "\n\n") for _, e := range entries { bb.WriteString(e + "\n") } @@ -140,7 +140,7 @@ func UpdateChangelog(c Config) error { return nil } -func UpdateMigrationGuide(c Config) error { +func updateMigrationGuide(c config) error { var bb bytes.Buffer existingFile, err := ioutil.ReadFile(c.File) if err != nil { @@ -155,7 +155,7 @@ func UpdateMigrationGuide(c Config) error { haveMigrations = true bb.WriteString(fmt.Sprintf("### %s\n\n", e.Migration.Header)) bb.WriteString(fmt.Sprintf("%s\n\n", strings.Trim(e.Migration.Body, "\n"))) - bb.WriteString(fmt.Sprintf("See %s for more details.\n\n", e.PullRequestLink())) + bb.WriteString(fmt.Sprintf("See %s for more details.\n\n", e.pullRequestLink())) } } if !haveMigrations { @@ -168,72 +168,72 @@ func UpdateMigrationGuide(c Config) error { return nil } -type Fragment struct { - Entries []Entry `yaml:"entries"` +type fragment struct { + Entries []entry `yaml:"entries"` } -type Entry struct { +type entry struct { Description string `yaml:"description"` - Kind EntryKind `yaml:"kind"` + Kind entryKind `yaml:"kind"` Breaking bool `yaml:"breaking"` - Migration *Migration `yaml:"migration,omitempty"` + Migration *migration `yaml:"migration,omitempty"` PullRequest *uint `yaml:"pull_request,omitempty"` } -type EntryKind string +type entryKind string const ( - Addition EntryKind = "addition" - Change EntryKind = "change" - Removal EntryKind = "removal" - Deprecation EntryKind = "deprecation" - Bugfix EntryKind = "bugfix" + addition entryKind = "addition" + change entryKind = "change" + removal entryKind = "removal" + deprecation entryKind = "deprecation" + bugfix entryKind = "bugfix" ) -func (k EntryKind) ToHeader() string { +func (k entryKind) toHeader() string { switch k { - case Addition: + case addition: return "### Additions" - case Change: + case change: return "### Changes" - case Removal: + case removal: return "### Removals" - case Deprecation: + case deprecation: return "### Deprecations" - case Bugfix: + case bugfix: return "### Bug Fixes" default: - panic("invalid entry kind; NOTE TO DEVELOPERS: check EntryKind.Validate") + panic("invalid entry kind; NOTE TO DEVELOPERS: check entryKind.validate") } } -type Migration struct { +type migration struct { Header string `yaml:"header"` Body string `yaml:"body"` } -type Config struct { +type config struct { File string Title string - Entries []Entry + Entries []entry } -func (f *Fragment) Validate() error { +func (f *fragment) validate() error { for i, e := range f.Entries { - if err := e.Validate(); err != nil { + if err := e.validate(); err != nil { return fmt.Errorf("entry[%d] invalid: %v", i, err) } } return nil } -func (e *Entry) Validate() error { - if err := e.Kind.Validate(); err != nil { +func (e *entry) validate() error { + if err := e.Kind.validate(); err != nil { return fmt.Errorf("invalid kind: %v", err) } - if e.Breaking && e.Kind != Change && e.Kind != Removal { - return fmt.Errorf("breaking changes can only be kind %q or %q, got %q", Change, Removal, e.Kind) + if e.Breaking && e.Kind != change && e.Kind != removal { + return fmt.Errorf("breaking changes can only be kind %q or %q, got %q", change, removal, e.Kind) } if e.Breaking && e.Migration == nil { @@ -241,34 +241,34 @@ func (e *Entry) Validate() error { } if e.Migration != nil { - if err := e.Migration.Validate(); err != nil { + if err := e.Migration.validate(); err != nil { return fmt.Errorf("invalid migration: %v", err) } } return nil } -func (e Entry) ToChangelogString() string { +func (e entry) toChangelogString() string { text := strings.Trim(e.Description, "\n") if e.Breaking { - text = fmt.Sprintf("**Breaking Change**: %s", text) + text = fmt.Sprintf("**Breaking change**: %s", text) } if !strings.HasSuffix(text, ".") && !strings.HasSuffix(text, "!") { text = fmt.Sprintf("%s.", text) } if e.PullRequest != nil { - text = fmt.Sprintf("%s (%s)", text, e.PullRequestLink()) + text = fmt.Sprintf("%s (%s)", text, e.pullRequestLink()) } return fmt.Sprintf("- %s", text) } -func (e Entry) PullRequestLink() string { +func (e entry) pullRequestLink() string { const repo = "github.com/operator-framework/operator-sdk" return fmt.Sprintf("[#%d](https://%s/pull/%d)", *e.PullRequest, repo, *e.PullRequest) } -func (k EntryKind) Validate() error { - for _, t := range []EntryKind{Addition, Change, Removal, Deprecation, Bugfix} { +func (k entryKind) validate() error { + for _, t := range []entryKind{addition, change, removal, deprecation, bugfix} { if k == t { return nil } @@ -276,7 +276,7 @@ func (k EntryKind) Validate() error { return fmt.Errorf("%q is not a supported kind", k) } -func (m Migration) Validate() error { +func (m migration) validate() error { if len(m.Header) == 0 { return errors.New("header not specified") } From a541f3ba60681200af75cbc8580475e027d5c634 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Thu, 2 Apr 2020 16:45:18 -0400 Subject: [PATCH 04/21] rename fragments/example.yaml to ensure PR discovery works through renames --- changelog/fragments/{example.yaml => example1.yaml} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/fragments/{example.yaml => example1.yaml} (100%) diff --git a/changelog/fragments/example.yaml b/changelog/fragments/example1.yaml similarity index 100% rename from changelog/fragments/example.yaml rename to changelog/fragments/example1.yaml From b872de452d285c40649ec897d6da77943eec000c Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Thu, 2 Apr 2020 17:28:58 -0400 Subject: [PATCH 05/21] support PR discovery --- changelog/fragments/example1.yaml | 3 --- changelog/fragments/example2.yaml | 25 ++++++++++++++++++++ changelog/generate.go | 39 ++++++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 changelog/fragments/example2.yaml diff --git a/changelog/fragments/example1.yaml b/changelog/fragments/example1.yaml index a6161503826..cdcf5cb86da 100644 --- a/changelog/fragments/example1.yaml +++ b/changelog/fragments/example1.yaml @@ -4,7 +4,6 @@ entries: It's a great change! kind: change breaking: false - pull_request: 3001 - description: > Here is another change from the same PR. Using '>' means @@ -12,7 +11,6 @@ entries: just one line. kind: change breaking: true - pull_request: 3001 migration: header: Breaking changes must have migrations body: > @@ -22,4 +20,3 @@ entries: - description: Here's a bug we fixed! kind: bugfix breaking: false - pull_request: 3003 \ No newline at end of file diff --git a/changelog/fragments/example2.yaml b/changelog/fragments/example2.yaml new file mode 100644 index 00000000000..a6161503826 --- /dev/null +++ b/changelog/fragments/example2.yaml @@ -0,0 +1,25 @@ +entries: + - description: > + This is the description for a change that was made. + It's a great change! + kind: change + breaking: false + pull_request: 3001 + + - description: > + Here is another change from the same PR. Using '>' means + I can span multiple lines in YAML and the parser sees it as + just one line. + kind: change + breaking: true + pull_request: 3001 + migration: + header: Breaking changes must have migrations + body: > + When auto-generating the migration guide, we expect a blurb + about every single breaking change. + + - description: Here's a bug we fixed! + kind: bugfix + breaking: false + pull_request: 3003 \ No newline at end of file diff --git a/changelog/generate.go b/changelog/generate.go index 5e35a953a0e..130c0677980 100644 --- a/changelog/generate.go +++ b/changelog/generate.go @@ -7,7 +7,10 @@ import ( "fmt" "io/ioutil" "os" + "os/exec" "path/filepath" + "regexp" + "strconv" "strings" "gopkg.in/yaml.v2" @@ -98,6 +101,19 @@ func loadEntries(fragmentsDir string) ([]entry, error) { return nil, fmt.Errorf("failed to validate fragment file %q: %w", fragFile.Name(), err) } + prNum, err := getPullRequest(path) + if err != nil { + log.Warnf("failed to get PR for fragment file %q: %v", fragFile.Name(), err) + } + + if prNum != 0 { + for i, e := range fragment.Entries { + if e.PullRequest == nil { + fragment.Entries[i].PullRequest = &prNum + } + } + } + entries = append(entries, fragment.Entries...) } return entries, nil @@ -155,7 +171,9 @@ func updateMigrationGuide(c config) error { haveMigrations = true bb.WriteString(fmt.Sprintf("### %s\n\n", e.Migration.Header)) bb.WriteString(fmt.Sprintf("%s\n\n", strings.Trim(e.Migration.Body, "\n"))) - bb.WriteString(fmt.Sprintf("See %s for more details.\n\n", e.pullRequestLink())) + if e.PullRequest != nil { + bb.WriteString(fmt.Sprintf("See %s for more details.\n\n", e.pullRequestLink())) + } } } if !haveMigrations { @@ -267,6 +285,25 @@ func (e entry) pullRequestLink() string { return fmt.Sprintf("[#%d](https://%s/pull/%d)", *e.PullRequest, repo, *e.PullRequest) } +func getPullRequest(filename string) (uint, error) { + args := fmt.Sprintf("log --follow --pretty=format:%%s --diff-filter=A --find-renames=40%% %s", filename) + line, err := exec.Command("git", strings.Split(args, " ")...).CombinedOutput() + if err != nil { + return 0, fmt.Errorf("failed to locate git commit for PR discovery: %v", err) + } + + numRegex := regexp.MustCompile(`\(#(\d+)\)$`) + matches := numRegex.FindAllStringSubmatch(string(line), 1) + if len(matches) == 0 || len(matches[0]) < 2 { + return 0, fmt.Errorf("could not find PR number in commit message") + } + u64, err := strconv.ParseUint(matches[0][1], 10, 64) + if err != nil { + return 0, fmt.Errorf("failed to parse PR number %q from commit message: %v", matches[0][1], err) + } + return uint(u64), nil +} + func (k entryKind) validate() error { for _, t := range []entryKind{addition, change, removal, deprecation, bugfix} { if k == t { From 0629fe40712c436c60a1c4c99f69ca7877dc8d74 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Fri, 3 Apr 2020 13:37:01 -0400 Subject: [PATCH 06/21] generate separate migration guide doc for each release --- changelog/generate.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/changelog/generate.go b/changelog/generate.go index 130c0677980..1a7af6c4372 100644 --- a/changelog/generate.go +++ b/changelog/generate.go @@ -23,7 +23,7 @@ func main() { title string fragmentsDir string changelogFile string - migrationFile string + migrationDir string ) flag.StringVar(&title, "title", "", @@ -32,9 +32,9 @@ func main() { "Path to changelog fragments directory") flag.StringVar(&changelogFile, "changelog", "CHANGELOG.md", "Path to CHANGELOG") - flag.StringVar(&migrationFile, "migration-guide", - filepath.Join("website", "content", "en", "docs", "migration", "version-upgrade-guide.md"), - "Path to migration guide") + flag.StringVar(&migrationDir, "migration-guide-dir", + filepath.Join("website", "content", "en", "docs", "migration"), + "Path to migration guide directory") flag.Parse() if title == "" { @@ -57,8 +57,8 @@ func main() { log.Fatalf("failed to update CHANGELOG: %v", err) } - if err := updateMigrationGuide(config{ - File: migrationFile, + if err := createMigrationGuide(config{ + File: filepath.Join(migrationDir, fmt.Sprintf("%s.md", title)), Title: title, Entries: entries, }); err != nil { @@ -156,23 +156,23 @@ func updateChangelog(c config) error { return nil } -func updateMigrationGuide(c config) error { +func createMigrationGuide(c config) error { var bb bytes.Buffer - existingFile, err := ioutil.ReadFile(c.File) - if err != nil { - return fmt.Errorf("could not read migration guide: %v", err) - } - bb.Write(bytes.Trim(existingFile, "\n")) - bb.WriteString(fmt.Sprintf("\n\n## %s\n\n", c.Title)) + bb.WriteString("---\n") + bb.WriteString(fmt.Sprintf("title: %s\n", c.Title)) + + // TODO: sort these according to semver? + bb.WriteString("weight: 12\n") + bb.WriteString("---\n\n") haveMigrations := false for _, e := range c.Entries { if e.Migration != nil { haveMigrations = true - bb.WriteString(fmt.Sprintf("### %s\n\n", e.Migration.Header)) + bb.WriteString(fmt.Sprintf("## %s\n\n", e.Migration.Header)) bb.WriteString(fmt.Sprintf("%s\n\n", strings.Trim(e.Migration.Body, "\n"))) if e.PullRequest != nil { - bb.WriteString(fmt.Sprintf("See %s for more details.\n\n", e.pullRequestLink())) + bb.WriteString(fmt.Sprintf("_See %s for more details._\n\n", e.pullRequestLink())) } } } From 04c8e04eb84e604bf8d47da4e495c7920025d7ad Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 6 Apr 2020 17:03:41 -0400 Subject: [PATCH 07/21] Add make target and generate script, update release doc --- Makefile | 3 ++ doc/dev/release.md | 31 +++++++------------ .../generate/gen-changelog.go | 14 ++++----- hack/generate/gen-changelog.sh | 9 ++++++ 4 files changed, 31 insertions(+), 26 deletions(-) rename changelog/generate.go => hack/generate/gen-changelog.go (97%) create mode 100755 hack/generate/gen-changelog.sh diff --git a/Makefile b/Makefile index 4a53fd06153..73bae8a7377 100644 --- a/Makefile +++ b/Makefile @@ -98,6 +98,9 @@ gen-cli-doc: ## Generate CLI documentation gen-test-framework: build/operator-sdk ## Run generate commands to update test/test-framework ./hack/generate/gen-test-framework.sh +gen-changelog: ## Generate CHANGELOG.md and migration guide updates + ./hack/generate/gen-changelog.sh + generate: gen-cli-doc gen-test-framework ## Run all generate targets .PHONY: generate gen-cli-doc gen-test-framework diff --git a/doc/dev/release.md b/doc/dev/release.md index 243c4f8f8b0..4631ac02290 100644 --- a/doc/dev/release.md +++ b/doc/dev/release.md @@ -2,7 +2,7 @@ Making an Operator SDK release involves: -- Updating `CHANGELOG.md`. +- Updating `CHANGELOG.md` and migration guide. - Tagging and signing a git commit and pushing the tag to GitHub. - Building a release binary and signing the binary - Creating a release by uploading binary, signature, and `CHANGELOG.md` updates for the release to GitHub. @@ -193,7 +193,7 @@ $ git push origin release-v1.3.1 Create a PR from `release-v1.3.1` to `v1.3.x`. Once CI passes and your PR is merged, continue to step 1. -### 1. Create a PR for release version and CHANGELOG.md updates +### 1. Create a PR for release version, CHANGELOG.md, and migration guide updates Once all PR's needed for a release have been merged, branch from `master`: @@ -215,14 +215,22 @@ Create a new branch to push release commits: $ git checkout -b release-v1.3.0 ``` +Run the CHANGELOG and migration guide generator: + +```sh +$ GEN_CHANGELOG_TAG=v1.3.0 make gen-changelog +``` + Commit the following changes: - `version/version.go`: update `Version` to `v1.3.0`. - `internal/scaffold/go_mod.go`, change the `require` line version for `github.com/operator-framework/operator-sdk` from `master` to `v1.3.0`. - `internal/scaffold/helm/go_mod.go`: same as for `internal/scaffold/go_mod.go`. - `internal/scaffold/ansible/go_mod.go`: same as for `internal/scaffold/go_mod.go`. -- `CHANGELOG.md`: update the `## Unreleased` header to `## v1.3.0`. - `doc/user/install-operator-sdk.md`: update the linux and macOS URLs to point to the new release URLs. +- `CHANGELOG.md`: commit changes (updated by changelog generation). +- `website/content/en/docs/migration/v1.3.0.md`: commit changes (created by changelog generation). +- `changelog/fragments/*`: commit deleted fragment files (deleted by changelog generation). _(Non-patch releases only)_ Lock down the master branch to prevent further commits between this and step 4. See [this section](#locking-down-branches) for steps to do so. @@ -254,7 +262,7 @@ Once this tag passes CI, go to step 3. For more info on tagging, see the [releas **Note:** If CI fails for some reason, you will have to revert the tagged commit, re-commit, and make a new PR. -### 3. Create a PR for post-release version and CHANGELOG.md updates +### 3. Create a PR for post-release version updates Check out a new branch from master (or use your `release-v1.3.0` branch) and commit the following changes: @@ -262,21 +270,6 @@ Check out a new branch from master (or use your `release-v1.3.0` branch) and com - `internal/scaffold/go_mod.go`, change the `require` line version for `github.com/operator-framework/operator-sdk` from `v1.3.0` to `master`. - `internal/scaffold/helm/go_mod.go`: same as for `internal/scaffold/go_mod.go`. - `internal/scaffold/ansible/go_mod.go`: same as for `internal/scaffold/go_mod.go`. -- `CHANGELOG.md`: add the following as a new set of headers above `## v1.3.0`: - - ```markdown - ## Unreleased - - ### Added - - ### Changed - - ### Deprecated - - ### Removed - - ### Bug Fixes - ``` Create a new PR for this branch, targetting the `master` branch. Once this PR passes CI and is merged, `master` can be unfrozen. diff --git a/changelog/generate.go b/hack/generate/gen-changelog.go similarity index 97% rename from changelog/generate.go rename to hack/generate/gen-changelog.go index 1a7af6c4372..d3a9de999f8 100644 --- a/changelog/generate.go +++ b/hack/generate/gen-changelog.go @@ -20,13 +20,13 @@ import ( func main() { var ( - title string + tag string fragmentsDir string changelogFile string migrationDir string ) - flag.StringVar(&title, "title", "", + flag.StringVar(&tag, "tag", "", "Title for generated CHANGELOG and migration guide sections") flag.StringVar(&fragmentsDir, "fragments-dir", filepath.Join("changelog", "fragments"), "Path to changelog fragments directory") @@ -37,8 +37,8 @@ func main() { "Path to migration guide directory") flag.Parse() - if title == "" { - log.Fatalf("flag '-title' is required!") + if tag == "" { + log.Fatalf("flag '-tag' is required!") } entries, err := loadEntries(fragmentsDir) @@ -51,15 +51,15 @@ func main() { if err := updateChangelog(config{ File: changelogFile, - Title: title, + Title: tag, Entries: entries, }); err != nil { log.Fatalf("failed to update CHANGELOG: %v", err) } if err := createMigrationGuide(config{ - File: filepath.Join(migrationDir, fmt.Sprintf("%s.md", title)), - Title: title, + File: filepath.Join(migrationDir, fmt.Sprintf("%s.md", tag)), + Title: tag, Entries: entries, }); err != nil { log.Fatalf("failed to update migration guide: %v", err) diff --git a/hack/generate/gen-changelog.sh b/hack/generate/gen-changelog.sh new file mode 100755 index 00000000000..3b019b569c0 --- /dev/null +++ b/hack/generate/gen-changelog.sh @@ -0,0 +1,9 @@ +#!/bin/bash + +source ./hack/lib/common.sh +set -e +shopt -s extglob + +[[ -n "$GEN_CHANGELOG_TAG" ]] || fatal "Must set GEN_CHANGELOG_TAG (e.g. export GEN_CHANGELOG_TAG=v1.2.3)" +go run ./hack/generate/gen-changelog.go -tag="${GEN_CHANGELOG_TAG}" +rm ./changelog/fragments/!(00-template.yaml) From cb71dfbbbd195112c58afda3ec479c14530a9ecf Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 6 Apr 2020 17:35:44 -0400 Subject: [PATCH 08/21] PR number is optional and for override purposes only --- changelog/fragments/00-template.yaml | 15 ++++++++++++--- hack/generate/gen-changelog.go | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/changelog/fragments/00-template.yaml b/changelog/fragments/00-template.yaml index 125cccda652..44f68b5ba65 100644 --- a/changelog/fragments/00-template.yaml +++ b/changelog/fragments/00-template.yaml @@ -3,7 +3,9 @@ entries: - description: > Description is the line that shows up in the CHANGELOG. This - should be formatted as markdown and be on a single line. + should be formatted as markdown and be on a single line. Using + the YAML string '>' operator means you can write your entry + multiple lines and it will still be parsed as a single line. # kind is one of: # - addition @@ -16,12 +18,19 @@ entries: # Is this a breaking change? breaking: false + # NOTE: ONLY USE `pull_request_override` WHEN ADDING THIS + # FILE FOR A PREVIOUSLY MERGED PULL_REQUEST! + # + # The generator auto-detects the PR number from the commit + # message in which this file was originally added. + # # What is the pull request number (without the "#")? - pull_request: 0 + # pull_request_override: 0 + # Migration can be defined to automatically add a section to # the migration guide. This is required for breaking changes. migration: header: Header text for the migration section body: > - Body of the migration section. \ No newline at end of file + Body of the migration section. diff --git a/hack/generate/gen-changelog.go b/hack/generate/gen-changelog.go index d3a9de999f8..d8bb529fa02 100644 --- a/hack/generate/gen-changelog.go +++ b/hack/generate/gen-changelog.go @@ -195,7 +195,7 @@ type entry struct { Kind entryKind `yaml:"kind"` Breaking bool `yaml:"breaking"` Migration *migration `yaml:"migration,omitempty"` - PullRequest *uint `yaml:"pull_request,omitempty"` + PullRequest *uint `yaml:"pull_request_override,omitempty"` } type entryKind string From 766b9297567764136640616dfb734ba13d3f9f17 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 6 Apr 2020 17:41:13 -0400 Subject: [PATCH 09/21] update fragment examples to use pull_request_override --- changelog/fragments/example2.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/changelog/fragments/example2.yaml b/changelog/fragments/example2.yaml index a6161503826..31ea349c61f 100644 --- a/changelog/fragments/example2.yaml +++ b/changelog/fragments/example2.yaml @@ -4,7 +4,7 @@ entries: It's a great change! kind: change breaking: false - pull_request: 3001 + pull_request_override: 3001 - description: > Here is another change from the same PR. Using '>' means @@ -12,7 +12,7 @@ entries: just one line. kind: change breaking: true - pull_request: 3001 + pull_request_override: 3001 migration: header: Breaking changes must have migrations body: > @@ -22,4 +22,4 @@ entries: - description: Here's a bug we fixed! kind: bugfix breaking: false - pull_request: 3003 \ No newline at end of file + pull_request_override: 3003 From 8eac47a6f9d9c280808174a605c0cd3393d23f8e Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 6 Apr 2020 18:03:29 -0400 Subject: [PATCH 10/21] move generate scripts to sub-directories to avoid multiple mains --- hack/generate/{ => changelog}/gen-changelog.go | 0 hack/generate/{ => changelog}/gen-changelog.sh | 2 +- hack/generate/{ => cli-doc}/gen-cli-doc.go | 0 hack/generate/cli-doc/gen-cli-doc.sh | 3 +++ hack/generate/gen-cli-doc.sh | 3 --- hack/generate/{ => test-framework}/gen-test-framework.sh | 0 hack/tests/sanity-check.sh | 4 ++-- 7 files changed, 6 insertions(+), 6 deletions(-) rename hack/generate/{ => changelog}/gen-changelog.go (100%) rename hack/generate/{ => changelog}/gen-changelog.sh (73%) rename hack/generate/{ => cli-doc}/gen-cli-doc.go (100%) create mode 100755 hack/generate/cli-doc/gen-cli-doc.sh delete mode 100755 hack/generate/gen-cli-doc.sh rename hack/generate/{ => test-framework}/gen-test-framework.sh (100%) diff --git a/hack/generate/gen-changelog.go b/hack/generate/changelog/gen-changelog.go similarity index 100% rename from hack/generate/gen-changelog.go rename to hack/generate/changelog/gen-changelog.go diff --git a/hack/generate/gen-changelog.sh b/hack/generate/changelog/gen-changelog.sh similarity index 73% rename from hack/generate/gen-changelog.sh rename to hack/generate/changelog/gen-changelog.sh index 3b019b569c0..f208cb400a8 100755 --- a/hack/generate/gen-changelog.sh +++ b/hack/generate/changelog/gen-changelog.sh @@ -5,5 +5,5 @@ set -e shopt -s extglob [[ -n "$GEN_CHANGELOG_TAG" ]] || fatal "Must set GEN_CHANGELOG_TAG (e.g. export GEN_CHANGELOG_TAG=v1.2.3)" -go run ./hack/generate/gen-changelog.go -tag="${GEN_CHANGELOG_TAG}" +go run ./hack/generate/changelog/gen-changelog.go -tag="${GEN_CHANGELOG_TAG}" rm ./changelog/fragments/!(00-template.yaml) diff --git a/hack/generate/gen-cli-doc.go b/hack/generate/cli-doc/gen-cli-doc.go similarity index 100% rename from hack/generate/gen-cli-doc.go rename to hack/generate/cli-doc/gen-cli-doc.go diff --git a/hack/generate/cli-doc/gen-cli-doc.sh b/hack/generate/cli-doc/gen-cli-doc.sh new file mode 100755 index 00000000000..5f789624694 --- /dev/null +++ b/hack/generate/cli-doc/gen-cli-doc.sh @@ -0,0 +1,3 @@ +#!/bin/bash + +go run ./hack/generate/cli-doc/gen-cli-doc.go diff --git a/hack/generate/gen-cli-doc.sh b/hack/generate/gen-cli-doc.sh deleted file mode 100755 index 5b716f55981..00000000000 --- a/hack/generate/gen-cli-doc.sh +++ /dev/null @@ -1,3 +0,0 @@ -#!/bin/bash - -go run ./hack/generate/gen-cli-doc.go diff --git a/hack/generate/gen-test-framework.sh b/hack/generate/test-framework/gen-test-framework.sh similarity index 100% rename from hack/generate/gen-test-framework.sh rename to hack/generate/test-framework/gen-test-framework.sh diff --git a/hack/tests/sanity-check.sh b/hack/tests/sanity-check.sh index 63a2ab14fdc..44299e0a939 100755 --- a/hack/tests/sanity-check.sh +++ b/hack/tests/sanity-check.sh @@ -8,8 +8,8 @@ go fmt ./... ./hack/check-license.sh ./hack/check-error-log-msg-format.sh ./hack/check-doc-diffs.sh -./hack/generate/gen-cli-doc.sh -./hack/generate/gen-test-framework.sh +./hack/generate/cli-doc/gen-cli-doc.sh +./hack/generate/test-framework/gen-test-framework.sh # Make sure repo is still in a clean state. git diff --exit-code From b375ad5c90d22b8bb2e07dc3596dd6cda4042645 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 6 Apr 2020 21:57:05 -0400 Subject: [PATCH 11/21] copy release doc changes to ./website --- .../docs/contribution-guidelines/release.md | 31 +++++++------------ 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/website/content/en/docs/contribution-guidelines/release.md b/website/content/en/docs/contribution-guidelines/release.md index 20f183f9905..631768683ed 100644 --- a/website/content/en/docs/contribution-guidelines/release.md +++ b/website/content/en/docs/contribution-guidelines/release.md @@ -5,7 +5,7 @@ weight: 30 Making an Operator SDK release involves: -- Updating `CHANGELOG.md`. +- Updating `CHANGELOG.md` and migration guide. - Tagging and signing a git commit and pushing the tag to GitHub. - Building a release binary and signing the binary - Creating a release by uploading binary, signature, and `CHANGELOG.md` updates for the release to GitHub. @@ -196,7 +196,7 @@ $ git push origin release-v1.3.1 Create a PR from `release-v1.3.1` to `v1.3.x`. Once CI passes and your PR is merged, continue to step 1. -### 1. Create a PR for release version and CHANGELOG.md updates +### 1. Create a PR for release version, CHANGELOG.md, and migration guide updates Once all PR's needed for a release have been merged, branch from `master`: @@ -218,14 +218,22 @@ Create a new branch to push release commits: $ git checkout -b release-v1.3.0 ``` +Run the CHANGELOG and migration guide generator: + +```sh +$ GEN_CHANGELOG_TAG=v1.3.0 make gen-changelog +``` + Commit the following changes: - `version/version.go`: update `Version` to `v1.3.0`. - `internal/scaffold/go_mod.go`, change the `require` line version for `github.com/operator-framework/operator-sdk` from `master` to `v1.3.0`. - `internal/scaffold/helm/go_mod.go`: same as for `internal/scaffold/go_mod.go`. - `internal/scaffold/ansible/go_mod.go`: same as for `internal/scaffold/go_mod.go`. -- `CHANGELOG.md`: update the `## Unreleased` header to `## v1.3.0`. - `doc/user/install-operator-sdk.md`: update the linux and macOS URLs to point to the new release URLs. +- `CHANGELOG.md`: commit changes (updated by changelog generation). +- `website/content/en/docs/migration/v1.3.0.md`: commit changes (created by changelog generation). +- `changelog/fragments/*`: commit deleted fragment files (deleted by changelog generation). _(Non-patch releases only)_ Lock down the master branch to prevent further commits between this and step 4. See [this section](#locking-down-branches) for steps to do so. @@ -257,7 +265,7 @@ Once this tag passes CI, go to step 3. For more info on tagging, see the [releas **Note:** If CI fails for some reason, you will have to revert the tagged commit, re-commit, and make a new PR. -### 3. Create a PR for post-release version and CHANGELOG.md updates +### 3. Create a PR for post-release version updates Check out a new branch from master (or use your `release-v1.3.0` branch) and commit the following changes: @@ -265,21 +273,6 @@ Check out a new branch from master (or use your `release-v1.3.0` branch) and com - `internal/scaffold/go_mod.go`, change the `require` line version for `github.com/operator-framework/operator-sdk` from `v1.3.0` to `master`. - `internal/scaffold/helm/go_mod.go`: same as for `internal/scaffold/go_mod.go`. - `internal/scaffold/ansible/go_mod.go`: same as for `internal/scaffold/go_mod.go`. -- `CHANGELOG.md`: add the following as a new set of headers above `## v1.3.0`: - - ```markdown - ## Unreleased - - ### Added - - ### Changed - - ### Deprecated - - ### Removed - - ### Bug Fixes - ``` Create a new PR for this branch, targetting the `master` branch. Once this PR passes CI and is merged, `master` can be unfrozen. From b39497c6d4897e71c04f9884b143e5c314f4d15b Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 6 Apr 2020 22:40:46 -0400 Subject: [PATCH 12/21] add validate-only flag --- hack/generate/changelog/gen-changelog.go | 11 +++++++++-- hack/tests/sanity-check.sh | 1 + 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/hack/generate/changelog/gen-changelog.go b/hack/generate/changelog/gen-changelog.go index d8bb529fa02..c8e32f0b5cd 100644 --- a/hack/generate/changelog/gen-changelog.go +++ b/hack/generate/changelog/gen-changelog.go @@ -24,6 +24,7 @@ func main() { fragmentsDir string changelogFile string migrationDir string + validateOnly bool ) flag.StringVar(&tag, "tag", "", @@ -35,10 +36,12 @@ func main() { flag.StringVar(&migrationDir, "migration-guide-dir", filepath.Join("website", "content", "en", "docs", "migration"), "Path to migration guide directory") + flag.BoolVar(&validateOnly, "validate-only", false, + "Only validate fragments") flag.Parse() - if tag == "" { - log.Fatalf("flag '-tag' is required!") + if tag == "" && !validateOnly { + log.Fatalf("flag '-tag' is required without '-validate-only'") } entries, err := loadEntries(fragmentsDir) @@ -49,6 +52,10 @@ func main() { log.Fatalf("no Entries found") } + if validateOnly { + return + } + if err := updateChangelog(config{ File: changelogFile, Title: tag, diff --git a/hack/tests/sanity-check.sh b/hack/tests/sanity-check.sh index 44299e0a939..b708c282936 100755 --- a/hack/tests/sanity-check.sh +++ b/hack/tests/sanity-check.sh @@ -10,6 +10,7 @@ go fmt ./... ./hack/check-doc-diffs.sh ./hack/generate/cli-doc/gen-cli-doc.sh ./hack/generate/test-framework/gen-test-framework.sh +go run ./hack/generate/changelog/gen-changelog.go -validate-only # Make sure repo is still in a clean state. git diff --exit-code From 966c1a82948087acc28e8bf185368d7bf483f3a5 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Thu, 9 Apr 2020 13:51:04 -0400 Subject: [PATCH 13/21] updated based on PR reviews --- hack/generate/changelog/gen-changelog.go | 68 +++++++++++++++--------- 1 file changed, 42 insertions(+), 26 deletions(-) diff --git a/hack/generate/changelog/gen-changelog.go b/hack/generate/changelog/gen-changelog.go index c8e32f0b5cd..2b34bb5da38 100644 --- a/hack/generate/changelog/gen-changelog.go +++ b/hack/generate/changelog/gen-changelog.go @@ -6,18 +6,21 @@ import ( "flag" "fmt" "io/ioutil" - "os" "os/exec" "path/filepath" "regexp" "strconv" "strings" - "gopkg.in/yaml.v2" - + "github.com/blang/semver" log "github.com/sirupsen/logrus" + "sigs.k8s.io/yaml" ) +var numRegex = regexp.MustCompile(`\(#(\d+)\)$`) + +const repo = "github.com/operator-framework/operator-sdk" + func main() { var ( tag string @@ -43,6 +46,13 @@ func main() { if tag == "" && !validateOnly { log.Fatalf("flag '-tag' is required without '-validate-only'") } + version, err := semver.Parse(strings.TrimPrefix(tag, "v")) + if err != nil { + log.Fatalf("flag '-tag' is not a valid semantic version: %v", err) + } + if len(version.Pre) > 0 || len(version.Build) > 0 { + log.Fatalf("flag '-tag' must not include a build number or pre-release identifiers") + } entries, err := loadEntries(fragmentsDir) if err != nil { @@ -58,18 +68,18 @@ func main() { if err := updateChangelog(config{ File: changelogFile, - Title: tag, + Version: version, Entries: entries, }); err != nil { log.Fatalf("failed to update CHANGELOG: %v", err) } if err := createMigrationGuide(config{ - File: filepath.Join(migrationDir, fmt.Sprintf("%s.md", tag)), - Title: tag, + File: filepath.Join(migrationDir, fmt.Sprintf("v%s.md", version)), + Version: version, Entries: entries, }); err != nil { - log.Fatalf("failed to update migration guide: %v", err) + log.Fatalf("failed to create migration guide: %v", err) } } @@ -88,19 +98,18 @@ func loadEntries(fragmentsDir string) ([]entry, error) { log.Warnf("Skipping directory %q", fragFile.Name()) continue } - if filepath.Ext(fragFile.Name()) != ".yaml" || fragFile.IsDir() { + if filepath.Ext(fragFile.Name()) != ".yaml" { log.Warnf("Skipping non-YAML file %q", fragFile.Name()) continue } path := filepath.Join(fragmentsDir, fragFile.Name()) - f, err := os.Open(path) + fragmentData, err := ioutil.ReadFile(path) if err != nil { - return nil, fmt.Errorf("failed to open fragment file %q: %w", fragFile.Name(), err) + return nil, fmt.Errorf("failed to read fragment file %q: %w", fragFile.Name(), err) } - decoder := yaml.NewDecoder(f) fragment := fragment{} - if err := decoder.Decode(&fragment); err != nil { + if err := yaml.Unmarshal(fragmentData, &fragment); err != nil { return nil, fmt.Errorf("failed to parse fragment file %q: %w", fragFile.Name(), err) } @@ -108,9 +117,13 @@ func loadEntries(fragmentsDir string) ([]entry, error) { return nil, fmt.Errorf("failed to validate fragment file %q: %w", fragFile.Name(), err) } - prNum, err := getPullRequest(path) + commitMsg, err := getCommitMessage(path) if err != nil { - log.Warnf("failed to get PR for fragment file %q: %v", fragFile.Name(), err) + log.Warnf("failed to get commit message for fragment file %q: %v", fragFile.Name(), err) + } + prNum, err := parsePRNumber(commitMsg) + if err != nil { + log.Warnf("failed to parse PR number for fragment file %q from string %q: %v", fragFile.Name(), commitMsg, err) } if prNum != 0 { @@ -140,7 +153,7 @@ func updateChangelog(c config) error { deprecation, bugfix, } - bb.WriteString(fmt.Sprintf("## %s\n\n", c.Title)) + bb.WriteString(fmt.Sprintf("## v%s\n\n", c.Version)) for _, k := range order { if entries, ok := changelog[k]; ok { bb.WriteString(k.toHeader() + "\n\n") @@ -167,10 +180,8 @@ func createMigrationGuide(c config) error { var bb bytes.Buffer bb.WriteString("---\n") - bb.WriteString(fmt.Sprintf("title: %s\n", c.Title)) - - // TODO: sort these according to semver? - bb.WriteString("weight: 12\n") + bb.WriteString(fmt.Sprintf("title: v%s\n", c.Version)) + bb.WriteString(fmt.Sprintf("weight: %d\n", convertVersionToWeight(c.Version))) bb.WriteString("---\n\n") haveMigrations := false for _, e := range c.Entries { @@ -193,6 +204,10 @@ func createMigrationGuide(c config) error { return nil } +func convertVersionToWeight(v semver.Version) uint64 { + return 1_000_000_000 - (v.Major * 1_000_000) - (v.Minor * 1_000) - v.Patch +} + type fragment struct { Entries []entry `yaml:"entries"` } @@ -239,7 +254,7 @@ type migration struct { type config struct { File string - Title string + Version semver.Version Entries []entry } @@ -274,7 +289,7 @@ func (e *entry) validate() error { } func (e entry) toChangelogString() string { - text := strings.Trim(e.Description, "\n") + text := strings.TrimSpace(e.Description) if e.Breaking { text = fmt.Sprintf("**Breaking change**: %s", text) } @@ -288,19 +303,20 @@ func (e entry) toChangelogString() string { } func (e entry) pullRequestLink() string { - const repo = "github.com/operator-framework/operator-sdk" return fmt.Sprintf("[#%d](https://%s/pull/%d)", *e.PullRequest, repo, *e.PullRequest) } -func getPullRequest(filename string) (uint, error) { +func getCommitMessage(filename string) (string, error) { args := fmt.Sprintf("log --follow --pretty=format:%%s --diff-filter=A --find-renames=40%% %s", filename) line, err := exec.Command("git", strings.Split(args, " ")...).CombinedOutput() if err != nil { - return 0, fmt.Errorf("failed to locate git commit for PR discovery: %v", err) + return "", fmt.Errorf("failed to locate git commit for PR discovery: %v", err) } + return string(line), nil +} - numRegex := regexp.MustCompile(`\(#(\d+)\)$`) - matches := numRegex.FindAllStringSubmatch(string(line), 1) +func parsePRNumber(msg string) (uint, error) { + matches := numRegex.FindAllStringSubmatch(msg, 1) if len(matches) == 0 || len(matches[0]) < 2 { return 0, fmt.Errorf("could not find PR number in commit message") } From 54cb58f2f8be9f8b14d413cb132fd60482fe4db0 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Fri, 10 Apr 2020 01:02:57 -0400 Subject: [PATCH 14/21] separate into library, use Go templates --- go.mod | 1 + go.sum | 1 + hack/generate/changelog/gen-changelog.go | 304 +----------------- hack/generate/changelog/util/changelog.go | 119 +++++++ hack/generate/changelog/util/fragment.go | 183 +++++++++++ .../changelog/util/migration-guide.go | 77 +++++ 6 files changed, 396 insertions(+), 289 deletions(-) create mode 100644 hack/generate/changelog/util/changelog.go create mode 100644 hack/generate/changelog/util/fragment.go create mode 100644 hack/generate/changelog/util/migration-guide.go diff --git a/go.mod b/go.mod index 86aee7be57f..bc9bea4808f 100644 --- a/go.mod +++ b/go.mod @@ -27,6 +27,7 @@ require ( github.com/pborman/uuid v1.2.0 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.5.1 + github.com/prometheus/common v0.9.1 github.com/rogpeppe/go-internal v1.5.0 github.com/rubenv/sql-migrate v0.0.0-20191025130928-9355dd04f4b3 // indirect github.com/sergi/go-diff v1.0.0 diff --git a/go.sum b/go.sum index c061e058ffe..8ef1ea05cac 100644 --- a/go.sum +++ b/go.sum @@ -99,6 +99,7 @@ github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuy github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4 h1:Hs82Z41s6SdL1CELW+XaDYmOH4hkBN4/N9og/AsOv7E= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= +github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d h1:UQZhZ2O0vMHr2cI+DC1Mbh0TJxzA3RcLoMsFw+aXw7E= github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho= github.com/aliyun/aliyun-oss-go-sdk v2.0.4+incompatible/go.mod h1:T/Aws4fEfogEE9v+HPhhw+CntffsBHJ8nXQCwKr0/g8= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= diff --git a/hack/generate/changelog/gen-changelog.go b/hack/generate/changelog/gen-changelog.go index 2b34bb5da38..c54a85b2617 100644 --- a/hack/generate/changelog/gen-changelog.go +++ b/hack/generate/changelog/gen-changelog.go @@ -1,24 +1,17 @@ package main import ( - "bytes" - "errors" "flag" "fmt" - "io/ioutil" - "os/exec" "path/filepath" - "regexp" - "strconv" "strings" + "github.com/operator-framework/operator-sdk/hack/generate/changelog/util" + "github.com/blang/semver" log "github.com/sirupsen/logrus" - "sigs.k8s.io/yaml" ) -var numRegex = regexp.MustCompile(`\(#(\d+)\)$`) - const repo = "github.com/operator-framework/operator-sdk" func main() { @@ -46,302 +39,35 @@ func main() { if tag == "" && !validateOnly { log.Fatalf("flag '-tag' is required without '-validate-only'") } - version, err := semver.Parse(strings.TrimPrefix(tag, "v")) - if err != nil { - log.Fatalf("flag '-tag' is not a valid semantic version: %v", err) - } - if len(version.Pre) > 0 || len(version.Build) > 0 { - log.Fatalf("flag '-tag' must not include a build number or pre-release identifiers") - } - entries, err := loadEntries(fragmentsDir) + entries, err := util.LoadEntries(fragmentsDir, repo) if err != nil { log.Fatalf("failed to load fragments: %v", err) } if len(entries) == 0 { - log.Fatalf("no Entries found") + log.Fatalf("no entries found") } if validateOnly { return } - if err := updateChangelog(config{ - File: changelogFile, - Version: version, - Entries: entries, - }); err != nil { - log.Fatalf("failed to update CHANGELOG: %v", err) - } - - if err := createMigrationGuide(config{ - File: filepath.Join(migrationDir, fmt.Sprintf("v%s.md", version)), - Version: version, - Entries: entries, - }); err != nil { - log.Fatalf("failed to create migration guide: %v", err) - } -} - -func loadEntries(fragmentsDir string) ([]entry, error) { - files, err := ioutil.ReadDir(fragmentsDir) - if err != nil { - return nil, fmt.Errorf("failed to read fragments directory: %w", err) - } - - var entries []entry - for _, fragFile := range files { - if fragFile.Name() == "00-template.yaml" { - continue - } - if fragFile.IsDir() { - log.Warnf("Skipping directory %q", fragFile.Name()) - continue - } - if filepath.Ext(fragFile.Name()) != ".yaml" { - log.Warnf("Skipping non-YAML file %q", fragFile.Name()) - continue - } - path := filepath.Join(fragmentsDir, fragFile.Name()) - fragmentData, err := ioutil.ReadFile(path) - if err != nil { - return nil, fmt.Errorf("failed to read fragment file %q: %w", fragFile.Name(), err) - } - - fragment := fragment{} - if err := yaml.Unmarshal(fragmentData, &fragment); err != nil { - return nil, fmt.Errorf("failed to parse fragment file %q: %w", fragFile.Name(), err) - } - - if err := fragment.validate(); err != nil { - return nil, fmt.Errorf("failed to validate fragment file %q: %w", fragFile.Name(), err) - } - - commitMsg, err := getCommitMessage(path) - if err != nil { - log.Warnf("failed to get commit message for fragment file %q: %v", fragFile.Name(), err) - } - prNum, err := parsePRNumber(commitMsg) - if err != nil { - log.Warnf("failed to parse PR number for fragment file %q from string %q: %v", fragFile.Name(), commitMsg, err) - } - - if prNum != 0 { - for i, e := range fragment.Entries { - if e.PullRequest == nil { - fragment.Entries[i].PullRequest = &prNum - } - } - } - - entries = append(entries, fragment.Entries...) - } - return entries, nil -} - -func updateChangelog(c config) error { - changelog := map[entryKind][]string{} - for _, e := range c.Entries { - changelog[e.Kind] = append(changelog[e.Kind], e.toChangelogString()) - } - - var bb bytes.Buffer - order := []entryKind{ - addition, - change, - removal, - deprecation, - bugfix, - } - bb.WriteString(fmt.Sprintf("## v%s\n\n", c.Version)) - for _, k := range order { - if entries, ok := changelog[k]; ok { - bb.WriteString(k.toHeader() + "\n\n") - for _, e := range entries { - bb.WriteString(e + "\n") - } - bb.WriteString("\n") - } - } - - existingFile, err := ioutil.ReadFile(c.File) - if err != nil { - return fmt.Errorf("could not read CHANGELOG: %v", err) - } - bb.Write(existingFile) - - if err := ioutil.WriteFile(c.File, bb.Bytes(), 0644); err != nil { - return fmt.Errorf("could not write CHANGELOG file: %v", err) - } - return nil -} - -func createMigrationGuide(c config) error { - var bb bytes.Buffer - - bb.WriteString("---\n") - bb.WriteString(fmt.Sprintf("title: v%s\n", c.Version)) - bb.WriteString(fmt.Sprintf("weight: %d\n", convertVersionToWeight(c.Version))) - bb.WriteString("---\n\n") - haveMigrations := false - for _, e := range c.Entries { - if e.Migration != nil { - haveMigrations = true - bb.WriteString(fmt.Sprintf("## %s\n\n", e.Migration.Header)) - bb.WriteString(fmt.Sprintf("%s\n\n", strings.Trim(e.Migration.Body, "\n"))) - if e.PullRequest != nil { - bb.WriteString(fmt.Sprintf("_See %s for more details._\n\n", e.pullRequestLink())) - } - } - } - if !haveMigrations { - bb.WriteString("There are no migrations for this release! :tada:\n\n") - } - - if err := ioutil.WriteFile(c.File, bytes.TrimSuffix(bb.Bytes(), []byte("\n")), 0644); err != nil { - return fmt.Errorf("could not write migration guide: %v", err) - } - return nil -} - -func convertVersionToWeight(v semver.Version) uint64 { - return 1_000_000_000 - (v.Major * 1_000_000) - (v.Minor * 1_000) - v.Patch -} - -type fragment struct { - Entries []entry `yaml:"entries"` -} - -type entry struct { - Description string `yaml:"description"` - Kind entryKind `yaml:"kind"` - Breaking bool `yaml:"breaking"` - Migration *migration `yaml:"migration,omitempty"` - PullRequest *uint `yaml:"pull_request_override,omitempty"` -} - -type entryKind string - -const ( - addition entryKind = "addition" - change entryKind = "change" - removal entryKind = "removal" - deprecation entryKind = "deprecation" - bugfix entryKind = "bugfix" -) - -func (k entryKind) toHeader() string { - switch k { - case addition: - return "### Additions" - case change: - return "### Changes" - case removal: - return "### Removals" - case deprecation: - return "### Deprecations" - case bugfix: - return "### Bug Fixes" - default: - panic("invalid entry kind; NOTE TO DEVELOPERS: check entryKind.validate") - } -} - -type migration struct { - Header string `yaml:"header"` - Body string `yaml:"body"` -} - -type config struct { - File string - Version semver.Version - Entries []entry -} - -func (f *fragment) validate() error { - for i, e := range f.Entries { - if err := e.validate(); err != nil { - return fmt.Errorf("entry[%d] invalid: %v", i, err) - } - } - return nil -} - -func (e *entry) validate() error { - if err := e.Kind.validate(); err != nil { - return fmt.Errorf("invalid kind: %v", err) - } - - if e.Breaking && e.Kind != change && e.Kind != removal { - return fmt.Errorf("breaking changes can only be kind %q or %q, got %q", change, removal, e.Kind) - } - - if e.Breaking && e.Migration == nil { - return fmt.Errorf("breaking changes require migration descriptions") - } - - if e.Migration != nil { - if err := e.Migration.validate(); err != nil { - return fmt.Errorf("invalid migration: %v", err) - } - } - return nil -} - -func (e entry) toChangelogString() string { - text := strings.TrimSpace(e.Description) - if e.Breaking { - text = fmt.Sprintf("**Breaking change**: %s", text) - } - if !strings.HasSuffix(text, ".") && !strings.HasSuffix(text, "!") { - text = fmt.Sprintf("%s.", text) - } - if e.PullRequest != nil { - text = fmt.Sprintf("%s (%s)", text, e.pullRequestLink()) - } - return fmt.Sprintf("- %s", text) -} - -func (e entry) pullRequestLink() string { - return fmt.Sprintf("[#%d](https://%s/pull/%d)", *e.PullRequest, repo, *e.PullRequest) -} - -func getCommitMessage(filename string) (string, error) { - args := fmt.Sprintf("log --follow --pretty=format:%%s --diff-filter=A --find-renames=40%% %s", filename) - line, err := exec.Command("git", strings.Split(args, " ")...).CombinedOutput() + version, err := semver.Parse(strings.TrimPrefix(tag, "v")) if err != nil { - return "", fmt.Errorf("failed to locate git commit for PR discovery: %v", err) - } - return string(line), nil -} - -func parsePRNumber(msg string) (uint, error) { - matches := numRegex.FindAllStringSubmatch(msg, 1) - if len(matches) == 0 || len(matches[0]) < 2 { - return 0, fmt.Errorf("could not find PR number in commit message") + log.Fatalf("flag '-tag' is not a valid semantic version: %v", err) } - u64, err := strconv.ParseUint(matches[0][1], 10, 64) - if err != nil { - return 0, fmt.Errorf("failed to parse PR number %q from commit message: %v", matches[0][1], err) + if len(version.Pre) > 0 || len(version.Build) > 0 { + log.Fatalf("flag '-tag' must not include a build number or pre-release identifiers") } - return uint(u64), nil -} -func (k entryKind) validate() error { - for _, t := range []entryKind{addition, change, removal, deprecation, bugfix} { - if k == t { - return nil - } + cl := util.ChangelogFromEntries(version, entries) + if err := cl.WriteFile(changelogFile); err != nil { + log.Fatalf("failed to update CHANGELOG: %v", err) } - return fmt.Errorf("%q is not a supported kind", k) -} -func (m migration) validate() error { - if len(m.Header) == 0 { - return errors.New("header not specified") - } - if len(m.Body) == 0 { - return errors.New("body not specified") + mg := util.MigrationGuideFromEntries(version, entries) + mgFile := filepath.Join(migrationDir, fmt.Sprintf("v%s.md", version)) + if err := mg.WriteFile(mgFile); err != nil { + log.Fatalf("failed to create migration guide: %v", err) } - return nil } diff --git a/hack/generate/changelog/util/changelog.go b/hack/generate/changelog/util/changelog.go new file mode 100644 index 00000000000..021680dabac --- /dev/null +++ b/hack/generate/changelog/util/changelog.go @@ -0,0 +1,119 @@ +package util + +import ( + "bytes" + "fmt" + "io/ioutil" + "strings" + "text/template" + + "github.com/blang/semver" +) + +type Changelog struct { + Version string + Additions []ChangelogEntry + Changes []ChangelogEntry + Removals []ChangelogEntry + Deprecations []ChangelogEntry + Bugfixes []ChangelogEntry + + Repo string +} + +type ChangelogEntry struct { + Description string + Link string +} + +const changelogTemplate = `## v{{ .Version }} +{{- with .Additions }} + +### Additions +{{ range . }} +- {{ .Description }} ({{ .Link }}) +{{- end }}{{- end }} +{{- with .Changes }} + +### Changes +{{ range . }} +- {{ .Description }} ({{ .Link }}) +{{- end }}{{- end }} +{{- with .Removals }} + +### Removals +{{ range . }} +- {{ .Description }} ({{ .Link }}) +{{- end }}{{- end }} +{{- with .Deprecations }} + +### Deprecations +{{ range . }} +- {{ .Description }} ({{ .Link }}) +{{- end }}{{- end }} +{{- with .Bugfixes }} + +### Bug Fixes +{{ range . }} +- {{ .Description }} ({{ .Link }}) +{{- end }}{{- end }}` + +var changelogTmpl = template.Must(template.New("changelog").Parse(changelogTemplate)) + +func (c *Changelog) Template() ([]byte, error) { + w := &bytes.Buffer{} + if err := changelogTmpl.Execute(w, c); err != nil { + return nil, err + } + return w.Bytes(), nil +} + +func (c *Changelog) WriteFile(path string) error { + data, err := c.Template() + if err != nil { + return err + } + existingFile, err := ioutil.ReadFile(path) + if err != nil { + return err + } + data = append(data, '\n', '\n') + data = append(data, existingFile...) + return ioutil.WriteFile(path, data, 0644) +} + +func ChangelogFromEntries(version semver.Version, entries []FragmentEntry) Changelog { + cl := Changelog{ + Version: version.String(), + } + for _, e := range entries { + cle := e.ToChangelogEntry() + switch e.Kind { + case Addition: + cl.Additions = append(cl.Additions, cle) + case Change: + cl.Changes = append(cl.Changes, cle) + case Removal: + cl.Removals = append(cl.Removals, cle) + case Deprecation: + cl.Deprecations = append(cl.Deprecations, cle) + case Bugfix: + cl.Bugfixes = append(cl.Bugfixes, cle) + } + } + return cl +} + +func (e *FragmentEntry) ToChangelogEntry() ChangelogEntry { + cle := ChangelogEntry{} + desc := strings.TrimSpace(e.Description) + if e.Breaking { + desc = fmt.Sprintf("**Breaking change**: %s", desc) + } + if !strings.HasSuffix(desc, ".") && !strings.HasSuffix(desc, "!") { + desc = fmt.Sprintf("%s.", desc) + } + cle.Description = desc + cle.Link = e.PullRequestLink + return cle +} diff --git a/hack/generate/changelog/util/fragment.go b/hack/generate/changelog/util/fragment.go new file mode 100644 index 00000000000..48e6930bfbe --- /dev/null +++ b/hack/generate/changelog/util/fragment.go @@ -0,0 +1,183 @@ +package util + +import ( + "errors" + "fmt" + "io/ioutil" + "os/exec" + "path/filepath" + "regexp" + "strconv" + "strings" + + "github.com/prometheus/common/log" + "gopkg.in/yaml.v2" +) + +type Fragment struct { + Entries []FragmentEntry `yaml:"entries"` +} + +func (f *Fragment) Validate() error { + for i, e := range f.Entries { + if err := e.Validate(); err != nil { + return fmt.Errorf("entry[%d] invalid: %v", i, err) + } + } + return nil +} + +type FragmentEntry struct { + Description string `yaml:"description"` + Kind EntryKind `yaml:"kind"` + Breaking bool `yaml:"breaking"` + Migration *EntryMigration `yaml:"migration,omitempty"` + PullRequest *uint `yaml:"pull_request_override,omitempty"` + + PullRequestLink string `yaml:"-"` +} + +func (e *FragmentEntry) Validate() error { + if err := e.Kind.Validate(); err != nil { + return fmt.Errorf("invalid kind: %v", err) + } + + if e.Breaking && e.Kind != Change && e.Kind != Removal { + return fmt.Errorf("breaking changes can only be kind %q or %q, got %q", Change, Removal, e.Kind) + } + + if e.Breaking && e.Migration == nil { + return fmt.Errorf("breaking changes require migration descriptions") + } + + if e.Migration != nil { + if err := e.Migration.Validate(); err != nil { + return fmt.Errorf("invalid migration: %v", err) + } + } + return nil +} + +func (e FragmentEntry) pullRequestLink(repo string) string { + if e.PullRequest == nil { + return "" + } + return fmt.Sprintf("[#%d](https://%s/pull/%d)", *e.PullRequest, repo, *e.PullRequest) +} + +type EntryKind string + +const ( + Addition EntryKind = "addition" + Change EntryKind = "change" + Removal EntryKind = "removal" + Deprecation EntryKind = "deprecation" + Bugfix EntryKind = "bugfix" +) + +func (k EntryKind) Validate() error { + for _, t := range []EntryKind{Addition, Change, Removal, Deprecation, Bugfix} { + if k == t { + return nil + } + } + return fmt.Errorf("%q is not a supported kind", k) +} + +type EntryMigration struct { + Header string `yaml:"header"` + Body string `yaml:"body"` +} + +func (m EntryMigration) Validate() error { + if len(m.Header) == 0 { + return errors.New("header not specified") + } + if len(m.Body) == 0 { + return errors.New("body not specified") + } + return nil +} + +func LoadEntries(fragmentsDir, repo string) ([]FragmentEntry, error) { + files, err := ioutil.ReadDir(fragmentsDir) + if err != nil { + return nil, fmt.Errorf("failed to read fragments directory: %w", err) + } + + var entries []FragmentEntry + for _, fragFile := range files { + if fragFile.Name() == "00-template.yaml" { + continue + } + if fragFile.IsDir() { + log.Warnf("Skipping directory %q", fragFile.Name()) + continue + } + if filepath.Ext(fragFile.Name()) != ".yaml" { + log.Warnf("Skipping non-YAML file %q", fragFile.Name()) + continue + } + path := filepath.Join(fragmentsDir, fragFile.Name()) + fragmentData, err := ioutil.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("failed to read fragment file %q: %w", fragFile.Name(), err) + } + + fragment := Fragment{} + if err := yaml.Unmarshal(fragmentData, &fragment); err != nil { + return nil, fmt.Errorf("failed to parse fragment file %q: %w", fragFile.Name(), err) + } + + if err := fragment.Validate(); err != nil { + return nil, fmt.Errorf("failed to validate fragment file %q: %w", fragFile.Name(), err) + } + + commitMsg, err := getCommitMessage(path) + if err != nil { + log.Warnf("failed to get commit message for fragment file %q: %v", fragFile.Name(), err) + } + prNum, err := parsePRNumber(commitMsg) + if err != nil { + log.Warnf("failed to parse PR number for fragment file %q from string %q: %v", fragFile.Name(), commitMsg, err) + } + + if prNum != 0 { + for i, e := range fragment.Entries { + if e.PullRequest == nil { + fragment.Entries[i].PullRequest = &prNum + } + } + } + + for i, e := range fragment.Entries { + fragment.Entries[i].PullRequestLink = e.pullRequestLink(repo) + } + + entries = append(entries, fragment.Entries...) + } + return entries, nil +} + +func getCommitMessage(filename string) (string, error) { + args := fmt.Sprintf("log --follow --pretty=format:%%s --diff-filter=A --find-renames=40%% %s", filename) + line, err := exec.Command("git", strings.Split(args, " ")...).CombinedOutput() + if err != nil { + return "", fmt.Errorf("failed to locate git commit for PR discovery: %v", err) + } + return string(line), nil +} + +var numRegex = regexp.MustCompile(`\(#(\d+)\)$`) + +func parsePRNumber(msg string) (uint, error) { + matches := numRegex.FindAllStringSubmatch(msg, 1) + if len(matches) == 0 || len(matches[0]) < 2 { + return 0, fmt.Errorf("could not find PR number in commit message") + } + u64, err := strconv.ParseUint(matches[0][1], 10, 64) + if err != nil { + return 0, fmt.Errorf("failed to parse PR number %q from commit message: %v", matches[0][1], err) + } + return uint(u64), nil +} diff --git a/hack/generate/changelog/util/migration-guide.go b/hack/generate/changelog/util/migration-guide.go new file mode 100644 index 00000000000..9afacacef8f --- /dev/null +++ b/hack/generate/changelog/util/migration-guide.go @@ -0,0 +1,77 @@ +package util + +import ( + "bytes" + "io/ioutil" + "strings" + "text/template" + + "github.com/blang/semver" +) + +type MigrationGuide struct { + Version string + Weight uint64 + Migrations []Migration +} + +type Migration struct { + Header string + Body string + PullRequestLink string +} + +const migrationGuideTemplate = `--- +title: v{{ .Version }} +weight: {{ .Weight }} +--- +{{- range .Migrations }} +## {{ .Header }} + +{{ .Body }} + +_See {{ .PullRequestLink }} for more details._ +{{ else }} +There are no migrations for this release! :tada: +{{- end }} +` + +var migrationGuideTmpl = template.Must(template.New("migrationGuide").Parse(migrationGuideTemplate)) + +func (mg *MigrationGuide) Template() ([]byte, error) { + w := &bytes.Buffer{} + if err := migrationGuideTmpl.Execute(w, mg); err != nil { + return nil, err + } + return w.Bytes(), nil +} + +func (mg *MigrationGuide) WriteFile(path string) error { + data, err := mg.Template() + if err != nil { + return err + } + return ioutil.WriteFile(path, data, 0644) +} + +func MigrationGuideFromEntries(version semver.Version, entries []FragmentEntry) MigrationGuide { + mg := MigrationGuide{ + Version: version.String(), + Weight: versionToWeight(version), + } + for _, e := range entries { + if e.Migration == nil { + continue + } + mg.Migrations = append(mg.Migrations, Migration{ + Header: e.Migration.Header, + Body: strings.TrimSpace(e.Migration.Body), + PullRequestLink: e.PullRequestLink, + }) + } + return mg +} + +func versionToWeight(v semver.Version) uint64 { + return 1_000_000_000 - (v.Major * 1_000_000) - (v.Minor * 1_000) - v.Patch +} From 6bbb65d568830f6dfc869cf0de88226e7fd1d20b Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Sat, 11 Apr 2020 22:44:20 -0400 Subject: [PATCH 15/21] unexport ToChangelogEntry --- hack/generate/changelog/util/changelog.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hack/generate/changelog/util/changelog.go b/hack/generate/changelog/util/changelog.go index 021680dabac..b9aeb66e9df 100644 --- a/hack/generate/changelog/util/changelog.go +++ b/hack/generate/changelog/util/changelog.go @@ -87,7 +87,7 @@ func ChangelogFromEntries(version semver.Version, entries []FragmentEntry) Chang Version: version.String(), } for _, e := range entries { - cle := e.ToChangelogEntry() + cle := e.toChangelogEntry() switch e.Kind { case Addition: cl.Additions = append(cl.Additions, cle) @@ -104,7 +104,7 @@ func ChangelogFromEntries(version semver.Version, entries []FragmentEntry) Chang return cl } -func (e *FragmentEntry) ToChangelogEntry() ChangelogEntry { +func (e *FragmentEntry) toChangelogEntry() ChangelogEntry { cle := ChangelogEntry{} desc := strings.TrimSpace(e.Description) if e.Breaking { From 808631eb2fbc8d72354472e7f71f805e1778c62e Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 13 Apr 2020 11:07:13 -0400 Subject: [PATCH 16/21] hack/generate/changelog: unit tests for changelog generation --- hack/generate/changelog/util/changelog.go | 30 +- .../generate/changelog/util/changelog_test.go | 532 ++++++++++++++++++ 2 files changed, 552 insertions(+), 10 deletions(-) create mode 100644 hack/generate/changelog/util/changelog_test.go diff --git a/hack/generate/changelog/util/changelog.go b/hack/generate/changelog/util/changelog.go index b9aeb66e9df..696ad0afa79 100644 --- a/hack/generate/changelog/util/changelog.go +++ b/hack/generate/changelog/util/changelog.go @@ -2,8 +2,10 @@ package util import ( "bytes" + "errors" "fmt" "io/ioutil" + "os" "strings" "text/template" @@ -26,37 +28,41 @@ type ChangelogEntry struct { Link string } -const changelogTemplate = `## v{{ .Version }} +const changelogTemplate = `## {{ .Version }} +{{- if or .Additions .Changes .Removals .Deprecations .Bugfixes -}} {{- with .Additions }} ### Additions {{ range . }} -- {{ .Description }} ({{ .Link }}) +- {{ .Description }}{{ if .Link }} ({{ .Link }}){{ end }} {{- end }}{{- end }} {{- with .Changes }} ### Changes {{ range . }} -- {{ .Description }} ({{ .Link }}) +- {{ .Description }}{{ if .Link }} ({{ .Link }}){{ end }} {{- end }}{{- end }} {{- with .Removals }} ### Removals {{ range . }} -- {{ .Description }} ({{ .Link }}) +- {{ .Description }}{{ if .Link }} ({{ .Link }}){{ end }} {{- end }}{{- end }} {{- with .Deprecations }} ### Deprecations {{ range . }} -- {{ .Description }} ({{ .Link }}) +- {{ .Description }}{{ if .Link }} ({{ .Link }}){{ end }} {{- end }}{{- end }} {{- with .Bugfixes }} ### Bug Fixes {{ range . }} -- {{ .Description }} ({{ .Link }}) -{{- end }}{{- end }}` +- {{ .Description }}{{ if .Link }} ({{ .Link }}){{ end }} +{{- end }}{{- end }}{{- else }} + +No changes for this version!{{ end }} +` var changelogTmpl = template.Must(template.New("changelog").Parse(changelogTemplate)) @@ -74,17 +80,21 @@ func (c *Changelog) WriteFile(path string) error { return err } existingFile, err := ioutil.ReadFile(path) - if err != nil { + if err != nil && !errors.Is(err, os.ErrNotExist) { return err } - data = append(data, '\n', '\n') + if errors.Is(err, os.ErrNotExist) || len(existingFile) == 0 { + return ioutil.WriteFile(path, data, 0644) + } + + data = append(data, '\n') data = append(data, existingFile...) return ioutil.WriteFile(path, data, 0644) } func ChangelogFromEntries(version semver.Version, entries []FragmentEntry) Changelog { cl := Changelog{ - Version: version.String(), + Version: fmt.Sprintf("v%s", version), } for _, e := range entries { cle := e.toChangelogEntry() diff --git a/hack/generate/changelog/util/changelog_test.go b/hack/generate/changelog/util/changelog_test.go new file mode 100644 index 00000000000..e12b41faed7 --- /dev/null +++ b/hack/generate/changelog/util/changelog_test.go @@ -0,0 +1,532 @@ +package util + +import ( + "fmt" + "io/ioutil" + "os" + "testing" + + "github.com/blang/semver" + + "github.com/stretchr/testify/assert" +) + +func getChangelogEntries(n int) []ChangelogEntry { + entries := make([]ChangelogEntry, n) + for i := 0; i < n; i++ { + entries[i] = ChangelogEntry{ + Description: fmt.Sprintf("Changelog entry description %d.", i), + Link: "[#999999](https://example.com/test/changelog/pulls/999999)", + } + } + return entries +} + +func TestChangelog_Template(t *testing.T) { + testCases := []struct { + name string + changelog Changelog + output string + }{ + { + name: "all with 1 entry", + changelog: Changelog{ + Version: "v999.999.999", + Additions: getChangelogEntries(1), + Changes: getChangelogEntries(1), + Removals: getChangelogEntries(1), + Deprecations: getChangelogEntries(1), + Bugfixes: getChangelogEntries(1), + }, + output: `## v999.999.999 + +### Additions + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Changes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Removals + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Deprecations + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Bug Fixes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +`, + }, + { + name: "all with 2 entries", + changelog: Changelog{ + Version: "v999.999.999", + Additions: getChangelogEntries(2), + Changes: getChangelogEntries(2), + Removals: getChangelogEntries(2), + Deprecations: getChangelogEntries(2), + Bugfixes: getChangelogEntries(2), + }, + output: `## v999.999.999 + +### Additions + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Changes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Removals + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Deprecations + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Bug Fixes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) +`, + }, + { + name: "no additions", + changelog: Changelog{ + Version: "v999.999.999", + Additions: nil, + Changes: getChangelogEntries(1), + Removals: getChangelogEntries(1), + Deprecations: getChangelogEntries(1), + Bugfixes: getChangelogEntries(1), + }, + output: `## v999.999.999 + +### Changes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Removals + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Deprecations + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Bug Fixes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +`, + }, + { + name: "no changes", + changelog: Changelog{ + Version: "v999.999.999", + Additions: getChangelogEntries(1), + Changes: nil, + Removals: getChangelogEntries(1), + Deprecations: getChangelogEntries(1), + Bugfixes: getChangelogEntries(1), + }, + output: `## v999.999.999 + +### Additions + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Removals + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Deprecations + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Bug Fixes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +`, + }, + { + name: "no removals", + changelog: Changelog{ + Version: "v999.999.999", + Additions: getChangelogEntries(1), + Changes: getChangelogEntries(1), + Removals: nil, + Deprecations: getChangelogEntries(1), + Bugfixes: getChangelogEntries(1), + }, + output: `## v999.999.999 + +### Additions + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Changes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Deprecations + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Bug Fixes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +`, + }, + { + name: "no deprecations", + changelog: Changelog{ + Version: "v999.999.999", + Additions: getChangelogEntries(1), + Changes: getChangelogEntries(1), + Removals: getChangelogEntries(1), + Deprecations: nil, + Bugfixes: getChangelogEntries(1), + }, + output: `## v999.999.999 + +### Additions + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Changes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Removals + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Bug Fixes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +`, + }, + { + name: "no bug fixes", + changelog: Changelog{ + Version: "v999.999.999", + Additions: getChangelogEntries(1), + Changes: getChangelogEntries(1), + Removals: getChangelogEntries(1), + Deprecations: getChangelogEntries(1), + Bugfixes: nil, + }, + output: `## v999.999.999 + +### Additions + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Changes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Removals + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Deprecations + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +`, + }, + { + name: "entry with no link", + changelog: Changelog{ + Version: "v999.999.999", + Additions: []ChangelogEntry{ + {Description: "Changelog entry description 0."}, + {Description: "Changelog entry description 1.", Link: "[#999999](https://example.com/test/changelog/pulls/999999)"}, + }, + }, + output: `## v999.999.999 + +### Additions + +- Changelog entry description 0. +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) +`, + }, + { + name: "no entries", + changelog: Changelog{ + Version: "v999.999.999", + Additions: nil, + Changes: nil, + Removals: nil, + Deprecations: nil, + Bugfixes: nil, + }, + output: `## v999.999.999 + +No changes for this version! +`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + d, err := tc.changelog.Template() + if err != nil { + t.Errorf("Got unexpected error: %v", err) + } + assert.Equal(t, tc.output, string(d)) + }) + } +} + +func TestChangelog_WriteFile(t *testing.T) { + + testCases := []struct { + name string + changelog Changelog + existingFile bool + existingFileContents string + output string + }{ + { + name: "non-existent file", + changelog: Changelog{ + Version: "v999.999.999", + Additions: getChangelogEntries(2), + Changes: getChangelogEntries(2), + Removals: getChangelogEntries(2), + Deprecations: getChangelogEntries(2), + Bugfixes: getChangelogEntries(2), + }, + output: `## v999.999.999 + +### Additions + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Changes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Removals + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Deprecations + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Bug Fixes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) +`, + }, + { + name: "empty file", + changelog: Changelog{ + Version: "v999.999.999", + Additions: getChangelogEntries(2), + Changes: getChangelogEntries(2), + Removals: getChangelogEntries(2), + Deprecations: getChangelogEntries(2), + Bugfixes: getChangelogEntries(2), + }, + existingFile: true, + output: `## v999.999.999 + +### Additions + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Changes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Removals + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Deprecations + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Bug Fixes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) +`, + }, + { + name: "existing file", + changelog: Changelog{ + Version: "v999.999.999", + Additions: getChangelogEntries(2), + Changes: getChangelogEntries(2), + Removals: getChangelogEntries(2), + Deprecations: getChangelogEntries(2), + Bugfixes: getChangelogEntries(2), + }, + existingFileContents: `## v999.999.998 + +### Bug Fixes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) +`, + output: `## v999.999.999 + +### Additions + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Changes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Removals + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Deprecations + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +### Bug Fixes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) + +## v999.999.998 + +### Bug Fixes + +- Changelog entry description 0. ([#999999](https://example.com/test/changelog/pulls/999999)) +- Changelog entry description 1. ([#999999](https://example.com/test/changelog/pulls/999999)) +`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tmpFile, err := ioutil.TempFile("", "go-test-changelog") + assert.NoError(t, err) + assert.NoError(t, tmpFile.Close()) + defer assert.NoError(t, os.Remove(tmpFile.Name())) + + if tc.existingFile || len(tc.existingFileContents) > 0 { + assert.NoError(t, ioutil.WriteFile(tmpFile.Name(), []byte(tc.existingFileContents), 0644)) + } + + assert.NoError(t, tc.changelog.WriteFile(tmpFile.Name())) + + d, err := ioutil.ReadFile(tmpFile.Name()) + assert.NoError(t, err) + assert.Equal(t, tc.output, string(d)) + }) + } +} + +func TestChangelog_ChangelogFromEntries(t *testing.T) { + testCases := []struct { + name string + version semver.Version + entries []FragmentEntry + changelog Changelog + }{ + { + name: "no entries", + version: semver.MustParse("999.999.999"), + changelog: Changelog{Version: "v999.999.999"}, + }, + { + name: "add periods to descriptions and breaking change prefix", + version: semver.MustParse("999.999.999"), + entries: []FragmentEntry{ + { + Description: "Changelog entry description 0", + Kind: Addition, + Breaking: false, + PullRequestLink: "[#999999](https://example.com/test/changelog/pulls/999999)", + }, + { + Description: "Changelog entry description 0", + Kind: Change, + Breaking: true, + }, + { + Description: "Changelog entry description 0", + Kind: Removal, + Breaking: true, + }, + { + Description: "Changelog entry description 0", + Kind: Deprecation, + Breaking: false, + }, + { + Description: "Changelog entry description 0", + Kind: Bugfix, + Breaking: false, + }, + }, + changelog: Changelog{ + Version: "v999.999.999", + Additions: []ChangelogEntry{ + { + Description: "Changelog entry description 0.", + Link: "[#999999](https://example.com/test/changelog/pulls/999999)", + }, + }, + Changes: []ChangelogEntry{ + { + Description: "**Breaking change**: Changelog entry description 0.", + }, + }, + Removals: []ChangelogEntry{ + { + Description: "**Breaking change**: Changelog entry description 0.", + }, + }, + Deprecations: []ChangelogEntry{ + { + Description: "Changelog entry description 0.", + }, + }, + Bugfixes: []ChangelogEntry{ + { + Description: "Changelog entry description 0.", + }, + }, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cl := ChangelogFromEntries(tc.version, tc.entries) + assert.Equal(t, tc.changelog, cl) + }) + } +} From 86feeb380eb5e3251a5a2034feae44bdf168f218 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 13 Apr 2020 21:18:01 -0400 Subject: [PATCH 17/21] hach/generate/changelog: add unit tests for fragment parsing and migration guide --- changelog/fragments/example1.yaml | 22 -- changelog/fragments/example2.yaml | 25 -- hack/generate/changelog/util/changelog.go | 2 +- .../generate/changelog/util/changelog_test.go | 2 +- hack/generate/changelog/util/fragment.go | 34 +- hack/generate/changelog/util/fragment_test.go | 372 ++++++++++++++++++ ...{migration-guide.go => migration_guide.go} | 14 +- .../changelog/util/migration_guide_test.go | 291 ++++++++++++++ .../util/testdata/ignore/00-template.yaml | 36 ++ .../ignore/more-fragments/ignored.yaml | 19 + .../util/testdata/ignore/non-yaml.txt | 0 .../testdata/invalid_entry/fragment1.yaml | 4 + .../util/testdata/invalid_yaml/fragment1.yaml | 18 + .../util/testdata/valid/fragment1.yaml | 19 + .../util/testdata/valid/fragment2.yaml | 24 ++ 15 files changed, 817 insertions(+), 65 deletions(-) delete mode 100644 changelog/fragments/example1.yaml delete mode 100644 changelog/fragments/example2.yaml create mode 100644 hack/generate/changelog/util/fragment_test.go rename hack/generate/changelog/util/{migration-guide.go => migration_guide.go} (90%) create mode 100644 hack/generate/changelog/util/migration_guide_test.go create mode 100644 hack/generate/changelog/util/testdata/ignore/00-template.yaml create mode 100644 hack/generate/changelog/util/testdata/ignore/more-fragments/ignored.yaml create mode 100644 hack/generate/changelog/util/testdata/ignore/non-yaml.txt create mode 100644 hack/generate/changelog/util/testdata/invalid_entry/fragment1.yaml create mode 100644 hack/generate/changelog/util/testdata/invalid_yaml/fragment1.yaml create mode 100644 hack/generate/changelog/util/testdata/valid/fragment1.yaml create mode 100644 hack/generate/changelog/util/testdata/valid/fragment2.yaml diff --git a/changelog/fragments/example1.yaml b/changelog/fragments/example1.yaml deleted file mode 100644 index cdcf5cb86da..00000000000 --- a/changelog/fragments/example1.yaml +++ /dev/null @@ -1,22 +0,0 @@ -entries: - - description: > - This is the description for a change that was made. - It's a great change! - kind: change - breaking: false - - - description: > - Here is another change from the same PR. Using '>' means - I can span multiple lines in YAML and the parser sees it as - just one line. - kind: change - breaking: true - migration: - header: Breaking changes must have migrations - body: > - When auto-generating the migration guide, we expect a blurb - about every single breaking change. - - - description: Here's a bug we fixed! - kind: bugfix - breaking: false diff --git a/changelog/fragments/example2.yaml b/changelog/fragments/example2.yaml deleted file mode 100644 index 31ea349c61f..00000000000 --- a/changelog/fragments/example2.yaml +++ /dev/null @@ -1,25 +0,0 @@ -entries: - - description: > - This is the description for a change that was made. - It's a great change! - kind: change - breaking: false - pull_request_override: 3001 - - - description: > - Here is another change from the same PR. Using '>' means - I can span multiple lines in YAML and the parser sees it as - just one line. - kind: change - breaking: true - pull_request_override: 3001 - migration: - header: Breaking changes must have migrations - body: > - When auto-generating the migration guide, we expect a blurb - about every single breaking change. - - - description: Here's a bug we fixed! - kind: bugfix - breaking: false - pull_request_override: 3003 diff --git a/hack/generate/changelog/util/changelog.go b/hack/generate/changelog/util/changelog.go index 696ad0afa79..b04d8fca164 100644 --- a/hack/generate/changelog/util/changelog.go +++ b/hack/generate/changelog/util/changelog.go @@ -61,7 +61,7 @@ const changelogTemplate = `## {{ .Version }} - {{ .Description }}{{ if .Link }} ({{ .Link }}){{ end }} {{- end }}{{- end }}{{- else }} -No changes for this version!{{ end }} +No changes for this release!{{ end }} ` var changelogTmpl = template.Must(template.New("changelog").Parse(changelogTemplate)) diff --git a/hack/generate/changelog/util/changelog_test.go b/hack/generate/changelog/util/changelog_test.go index e12b41faed7..ab8c6a7364b 100644 --- a/hack/generate/changelog/util/changelog_test.go +++ b/hack/generate/changelog/util/changelog_test.go @@ -273,7 +273,7 @@ func TestChangelog_Template(t *testing.T) { }, output: `## v999.999.999 -No changes for this version! +No changes for this release! `, }, } diff --git a/hack/generate/changelog/util/fragment.go b/hack/generate/changelog/util/fragment.go index 48e6930bfbe..035d0b59888 100644 --- a/hack/generate/changelog/util/fragment.go +++ b/hack/generate/changelog/util/fragment.go @@ -42,12 +42,16 @@ func (e *FragmentEntry) Validate() error { return fmt.Errorf("invalid kind: %v", err) } + if len(e.Description) == 0 { + return errors.New("missing description") + } + if e.Breaking && e.Kind != Change && e.Kind != Removal { return fmt.Errorf("breaking changes can only be kind %q or %q, got %q", Change, Removal, e.Kind) } if e.Breaking && e.Migration == nil { - return fmt.Errorf("breaking changes require migration descriptions") + return fmt.Errorf("breaking changes require migration sections") } if e.Migration != nil { @@ -133,13 +137,9 @@ func LoadEntries(fragmentsDir, repo string) ([]FragmentEntry, error) { return nil, fmt.Errorf("failed to validate fragment file %q: %w", fragFile.Name(), err) } - commitMsg, err := getCommitMessage(path) - if err != nil { - log.Warnf("failed to get commit message for fragment file %q: %v", fragFile.Name(), err) - } - prNum, err := parsePRNumber(commitMsg) + prNum, err := prGetter.GetPullRequestNumberFor(path) if err != nil { - log.Warnf("failed to parse PR number for fragment file %q from string %q: %v", fragFile.Name(), commitMsg, err) + log.Warn(err) } if prNum != 0 { @@ -159,7 +159,23 @@ func LoadEntries(fragmentsDir, repo string) ([]FragmentEntry, error) { return entries, nil } -func getCommitMessage(filename string) (string, error) { +var prGetter PullRequestNumberGetter = &gitPullRequestNumberGetter{} + +type PullRequestNumberGetter interface { + GetPullRequestNumberFor(file string) (uint, error) +} + +type gitPullRequestNumberGetter struct{} + +func (g *gitPullRequestNumberGetter) GetPullRequestNumberFor(filename string) (uint, error) { + msg, err := g.getCommitMessage(filename) + if err != nil { + return 0, err + } + return g.parsePRNumber(msg) +} + +func (g *gitPullRequestNumberGetter) getCommitMessage(filename string) (string, error) { args := fmt.Sprintf("log --follow --pretty=format:%%s --diff-filter=A --find-renames=40%% %s", filename) line, err := exec.Command("git", strings.Split(args, " ")...).CombinedOutput() if err != nil { @@ -170,7 +186,7 @@ func getCommitMessage(filename string) (string, error) { var numRegex = regexp.MustCompile(`\(#(\d+)\)$`) -func parsePRNumber(msg string) (uint, error) { +func (g *gitPullRequestNumberGetter) parsePRNumber(msg string) (uint, error) { matches := numRegex.FindAllStringSubmatch(msg, 1) if len(matches) == 0 || len(matches[0]) < 2 { return 0, fmt.Errorf("could not find PR number in commit message") diff --git a/hack/generate/changelog/util/fragment_test.go b/hack/generate/changelog/util/fragment_test.go new file mode 100644 index 00000000000..aa299b25554 --- /dev/null +++ b/hack/generate/changelog/util/fragment_test.go @@ -0,0 +1,372 @@ +package util + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +type mockValidPRGetter struct{} + +var _ PullRequestNumberGetter = &mockValidPRGetter{} + +func (m *mockValidPRGetter) GetPullRequestNumberFor(file string) (uint, error) { + return 999998, nil +} + +func TestFragment_LoadEntries(t *testing.T) { + discoveredPRNum := uint(999998) + overriddenPRNum := uint(999999) + repoLink := "example.com/test/changelog" + + testCases := []struct { + name string + fragmentsDir string + prGetter PullRequestNumberGetter + expectedEntries []FragmentEntry + expectedErr string + }{ + { + name: "ignore non-fragments", + fragmentsDir: "testdata/ignore", + expectedEntries: nil, + }, + { + name: "invalid yaml", + fragmentsDir: "testdata/invalid_yaml", + expectedEntries: nil, + expectedErr: "unmarshal errors", + }, + { + name: "invalid entry", + fragmentsDir: "testdata/invalid_entry", + expectedEntries: nil, + expectedErr: `failed to validate fragment file`, + }, + { + name: "valid fragments", + fragmentsDir: "testdata/valid", + prGetter: &mockValidPRGetter{}, + expectedEntries: []FragmentEntry{ + { + Description: "Addition description 0", + Kind: Addition, + PullRequest: &discoveredPRNum, + PullRequestLink: fmt.Sprintf("[#%d](https://%s/pull/%d)", discoveredPRNum, repoLink, discoveredPRNum), + }, + { + Description: "Change description 0", + Kind: Change, + PullRequest: &discoveredPRNum, + PullRequestLink: fmt.Sprintf("[#%d](https://%s/pull/%d)", discoveredPRNum, repoLink, discoveredPRNum), + }, + { + Description: "Removal description 0", + Kind: Removal, + Breaking: true, + Migration: &EntryMigration{ + Header: "Header for removal migration 0", + Body: "Body for removal migration 0", + }, + PullRequest: &discoveredPRNum, + PullRequestLink: fmt.Sprintf("[#%d](https://%s/pull/%d)", discoveredPRNum, repoLink, discoveredPRNum), + }, + { + Description: "Deprecation description 0", + Kind: Deprecation, + PullRequest: &discoveredPRNum, + PullRequestLink: fmt.Sprintf("[#%d](https://%s/pull/%d)", discoveredPRNum, repoLink, discoveredPRNum), + }, + { + Description: "Bugfix description 0", + Kind: Bugfix, + PullRequest: &discoveredPRNum, + PullRequestLink: fmt.Sprintf("[#%d](https://%s/pull/%d)", discoveredPRNum, repoLink, discoveredPRNum), + }, + { + Description: "Addition description 1", + Kind: Addition, + PullRequest: &overriddenPRNum, + PullRequestLink: fmt.Sprintf("[#%d](https://%s/pull/%d)", overriddenPRNum, repoLink, overriddenPRNum), + }, + { + Description: "Change description 1", + Kind: Change, + PullRequest: &overriddenPRNum, + PullRequestLink: fmt.Sprintf("[#%d](https://%s/pull/%d)", overriddenPRNum, repoLink, overriddenPRNum), + }, + { + Description: "Removal description 1", + Kind: Removal, + Breaking: true, + Migration: &EntryMigration{ + Header: "Header for removal migration 1", + Body: "Body for removal migration 1", + }, + PullRequest: &overriddenPRNum, + PullRequestLink: fmt.Sprintf("[#%d](https://%s/pull/%d)", overriddenPRNum, repoLink, overriddenPRNum), + }, + { + Description: "Deprecation description 1", + Kind: Deprecation, + PullRequest: &overriddenPRNum, + PullRequestLink: fmt.Sprintf("[#%d](https://%s/pull/%d)", overriddenPRNum, repoLink, overriddenPRNum), + }, + { + Description: "Bugfix description 1", + Kind: Bugfix, + PullRequest: &overriddenPRNum, + PullRequestLink: fmt.Sprintf("[#%d](https://%s/pull/%d)", overriddenPRNum, repoLink, overriddenPRNum), + }, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + prGetter = tc.prGetter + entries, err := LoadEntries(tc.fragmentsDir, repoLink) + assert.Equal(t, tc.expectedEntries, entries) + if len(tc.expectedErr) > 0 { + if !strings.Contains(err.Error(), tc.expectedErr) { + t.Errorf("expected error to contain: %q, got %q", tc.expectedErr, err) + } + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestFragmentEntry_Validate(t *testing.T) { + testCases := []struct { + name string + fragmentEntry FragmentEntry + expectedErr string + }{ + { + name: "invalid kind", + fragmentEntry: FragmentEntry{ + Description: "description", + Kind: "invalid", + Breaking: false, + }, + expectedErr: "invalid kind", + }, + { + name: "missing description", + fragmentEntry: FragmentEntry{ + Description: "", + Kind: Addition, + Breaking: false, + }, + expectedErr: "missing description", + }, + { + name: "breaking addition not allowed", + fragmentEntry: FragmentEntry{ + Description: "description", + Kind: Addition, + Breaking: true, + }, + expectedErr: `breaking changes can only be kind "change" or "removal", got "addition"`, + }, + { + name: "breaking deprecation not allowed", + fragmentEntry: FragmentEntry{ + Description: "description", + Kind: Deprecation, + Breaking: true, + }, + expectedErr: `breaking changes can only be kind "change" or "removal", got "deprecation"`, + }, + { + name: "breaking bugfix not allowed", + fragmentEntry: FragmentEntry{ + Description: "description", + Kind: Bugfix, + Breaking: true, + }, + expectedErr: `breaking changes can only be kind "change" or "removal", got "bugfix"`, + }, + { + name: "migration missing", + fragmentEntry: FragmentEntry{ + Description: "description", + Kind: Change, + Breaking: true, + }, + expectedErr: `breaking changes require migration sections`, + }, + { + name: "migration header missing", + fragmentEntry: FragmentEntry{ + Description: "description", + Kind: Change, + Breaking: true, + Migration: &EntryMigration{ + Header: "", + Body: "migration body", + }, + }, + expectedErr: `invalid migration: header not specified`, + }, + { + name: "migration body missing", + fragmentEntry: FragmentEntry{ + Description: "description", + Kind: Change, + Breaking: true, + Migration: &EntryMigration{ + Header: "migration header", + Body: "", + }, + }, + expectedErr: `invalid migration: body not specified`, + }, + { + name: "breaking change allowed", + fragmentEntry: FragmentEntry{ + Description: "description", + Kind: Change, + Breaking: true, + Migration: &EntryMigration{ + Header: "migration header", + Body: "migration body", + }, + }, + expectedErr: ``, + }, + { + name: "breaking removal allowed", + fragmentEntry: FragmentEntry{ + Description: "description", + Kind: Removal, + Breaking: true, + Migration: &EntryMigration{ + Header: "migration header", + Body: "migration body", + }, + }, + expectedErr: ``, + }, + { + name: "non-breaking migration allowed", + fragmentEntry: FragmentEntry{ + Description: "description", + Kind: Addition, + Breaking: false, + Migration: &EntryMigration{ + Header: "migration header", + Body: "migration body", + }, + }, + expectedErr: ``, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.fragmentEntry.Validate() + + if len(tc.expectedErr) == 0 { + assert.NoError(t, err) + } else if err == nil { + t.Errorf("expected error to contain %q, got no error", tc.expectedErr) + } else { + if !strings.Contains(err.Error(), tc.expectedErr) { + t.Errorf("expected error to contain: %q, got %q", tc.expectedErr, err) + } + } + }) + } +} + +func TestFragmentEntry_PullRequestLink(t *testing.T) { + prNum := uint(999999) + testCases := []struct { + name string + fragmentEntry FragmentEntry + repo string + link string + }{ + { + name: "no link", + fragmentEntry: FragmentEntry{}, + link: "", + }, + { + name: "link with repo 1", + repo: "example.com/test/repo1", + fragmentEntry: FragmentEntry{PullRequest: &prNum}, + link: "[#999999](https://example.com/test/repo1/pull/999999)", + }, + { + name: "link with repo 2", + repo: "example.com/test/repo2", + fragmentEntry: FragmentEntry{PullRequest: &prNum}, + link: "[#999999](https://example.com/test/repo2/pull/999999)", + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + link := tc.fragmentEntry.pullRequestLink(tc.repo) + assert.Equal(t, tc.link, link) + }) + } +} + +func TestGitPullRequestNumberGetter_parsePRNumber(t *testing.T) { + prg := &gitPullRequestNumberGetter{} + testCases := []struct { + name string + msg string + prNum uint + expectedErr string + }{ + { + name: "valid message", + msg: "this is a message with a PR number at the end (#999999)", + prNum: uint(999999), + }, + { + name: "missing parentheses", + msg: "this is a message with a PR number at the end #999999", + expectedErr: `could not find PR number in commit message`, + }, + { + name: "not at the end", + msg: "this is a message with a PR number (#999999) in the middle", + expectedErr: `could not find PR number in commit message`, + }, + { + name: "no PR number", + msg: "this is a message without a PR number", + expectedErr: `could not find PR number in commit message`, + }, + { + name: "invalid PR number", + msg: "this is a message with a really big PR number (#99999999999999999999999999999999999999)", + expectedErr: `value out of range`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + prNum, err := prg.parsePRNumber(tc.msg) + + if len(tc.expectedErr) == 0 { + assert.NoError(t, err) + } else if err == nil { + t.Errorf("expected error to contain %q, got no error", tc.expectedErr) + } else { + if !strings.Contains(err.Error(), tc.expectedErr) { + t.Errorf("expected error to contain: %q, got %q", tc.expectedErr, err) + } + } + + assert.Equal(t, tc.prNum, prNum) + }) + } +} diff --git a/hack/generate/changelog/util/migration-guide.go b/hack/generate/changelog/util/migration_guide.go similarity index 90% rename from hack/generate/changelog/util/migration-guide.go rename to hack/generate/changelog/util/migration_guide.go index 9afacacef8f..b123c9df6de 100644 --- a/hack/generate/changelog/util/migration-guide.go +++ b/hack/generate/changelog/util/migration_guide.go @@ -2,6 +2,7 @@ package util import ( "bytes" + "fmt" "io/ioutil" "strings" "text/template" @@ -22,19 +23,18 @@ type Migration struct { } const migrationGuideTemplate = `--- -title: v{{ .Version }} +title: {{ .Version }} weight: {{ .Weight }} --- -{{- range .Migrations }} +{{ range .Migrations }} ## {{ .Header }} {{ .Body }} - +{{ if .PullRequestLink }} _See {{ .PullRequestLink }} for more details._ -{{ else }} +{{ end }}{{ else }} There are no migrations for this release! :tada: -{{- end }} -` +{{ end }}` var migrationGuideTmpl = template.Must(template.New("migrationGuide").Parse(migrationGuideTemplate)) @@ -56,7 +56,7 @@ func (mg *MigrationGuide) WriteFile(path string) error { func MigrationGuideFromEntries(version semver.Version, entries []FragmentEntry) MigrationGuide { mg := MigrationGuide{ - Version: version.String(), + Version: fmt.Sprintf("v%s", version.String()), Weight: versionToWeight(version), } for _, e := range entries { diff --git a/hack/generate/changelog/util/migration_guide_test.go b/hack/generate/changelog/util/migration_guide_test.go new file mode 100644 index 00000000000..d7db60fe523 --- /dev/null +++ b/hack/generate/changelog/util/migration_guide_test.go @@ -0,0 +1,291 @@ +package util + +import ( + "fmt" + "io/ioutil" + "os" + "testing" + + "github.com/blang/semver" + + "github.com/stretchr/testify/assert" +) + +func getMigrations(n int) []Migration { + sections := make([]Migration, n) + for i := 0; i < n; i++ { + sections[i] = Migration{ + Header: fmt.Sprintf("Migration header %d", i), + Body: fmt.Sprintf("Migration body %d", i), + PullRequestLink: "[#999999](https://example.com/test/changelog/pull/999999)", + } + } + return sections +} + +func TestMigrationGuide_Template(t *testing.T) { + testCases := []struct { + name string + mg MigrationGuide + output string + }{ + { + name: "link_then_no_link", + mg: MigrationGuide{ + Version: "v999.999.999", + Weight: 1, + Migrations: []Migration{ + { + Header: "Migration header 0", + Body: "Migration body 0", + PullRequestLink: "[#999999](https://example.com/test/changelog/pull/999999)", + }, + { + Header: "Migration header 1", + Body: "Migration body 1", + }, + }, + }, + output: `--- +title: v999.999.999 +weight: 1 +--- + +## Migration header 0 + +Migration body 0 + +_See [#999999](https://example.com/test/changelog/pull/999999) for more details._ + +## Migration header 1 + +Migration body 1 +`, + }, + { + name: "no_link_then_link", + mg: MigrationGuide{ + Version: "v999.999.999", + Weight: 2, + Migrations: []Migration{ + { + Header: "Migration header 0", + Body: "Migration body 0", + }, + { + Header: "Migration header 1", + Body: "Migration body 1", + PullRequestLink: "[#999999](https://example.com/test/changelog/pull/999999)", + }, + }, + }, + output: `--- +title: v999.999.999 +weight: 2 +--- + +## Migration header 0 + +Migration body 0 + +## Migration header 1 + +Migration body 1 + +_See [#999999](https://example.com/test/changelog/pull/999999) for more details._ +`, + }, + { + name: "no migrations", + mg: MigrationGuide{ + Version: "v999.999.999", + Weight: 3, + }, + output: `--- +title: v999.999.999 +weight: 3 +--- + +There are no migrations for this release! :tada: +`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + d, err := tc.mg.Template() + if err != nil { + t.Errorf("Got unexpected error: %v", err) + } + assert.Equal(t, tc.output, string(d)) + }) + } +} + +func TestMigrationGuide_WriteFile(t *testing.T) { + + testCases := []struct { + name string + mg MigrationGuide + output string + }{ + { + name: "valid", + mg: MigrationGuide{ + Version: "v999.999.999", + Migrations: []Migration{ + { + Header: "Migration header 0", + Body: "Migration body 0", + PullRequestLink: "[#999999](https://example.com/test/changelog/pull/999999)", + }, + { + Header: "Migration header 1", + Body: "Migration body 1", + }, + }, + }, + output: `--- +title: v999.999.999 +weight: 0 +--- + +## Migration header 0 + +Migration body 0 + +_See [#999999](https://example.com/test/changelog/pull/999999) for more details._ + +## Migration header 1 + +Migration body 1 +`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tmpFile, err := ioutil.TempFile("", "go-test-changelog") + assert.NoError(t, err) + assert.NoError(t, tmpFile.Close()) + defer assert.NoError(t, os.Remove(tmpFile.Name())) + + assert.NoError(t, tc.mg.WriteFile(tmpFile.Name())) + + d, err := ioutil.ReadFile(tmpFile.Name()) + assert.NoError(t, err) + assert.Equal(t, tc.output, string(d)) + }) + } +} + +func TestMigrationGuide_MigrationGuideFromEntries(t *testing.T) { + testCases := []struct { + name string + version semver.Version + entries []FragmentEntry + mg MigrationGuide + }{ + { + name: "no entries, weight 1", + version: semver.MustParse("999.999.999"), + mg: MigrationGuide{ + Version: "v999.999.999", + Weight: 1, + }, + }, + { + name: "no entries, weight 2", + version: semver.MustParse("999.999.998"), + mg: MigrationGuide{ + Version: "v999.999.998", + Weight: 2, + }, + }, + { + name: "no entries, weight 998_997_997", + version: semver.MustParse("1.2.3"), + mg: MigrationGuide{ + Version: "v1.2.3", + Weight: 998_997_997, + }, + }, + { + name: "no entries, weight 2", + version: semver.MustParse("3.2.1"), + mg: MigrationGuide{ + Version: "v3.2.1", + Weight: 996_997_999, + }, + }, + { + name: "some migrations", + version: semver.MustParse("999.999.999"), + entries: []FragmentEntry{ + { + Description: "Changelog entry description 0", + Kind: Addition, + Breaking: false, + PullRequestLink: "[#999999](https://example.com/test/changelog/pulls/999999)", + Migration: &EntryMigration{ + Header: "Migration header 0", + Body: "Migration body 0", + }, + }, + { + Description: "Changelog entry description 1", + Kind: Change, + Breaking: true, + Migration: &EntryMigration{ + Header: "Migration header 1", + Body: "Migration body 1", + }, + }, + { + Description: "Changelog entry description 2", + Kind: Removal, + Breaking: true, + Migration: &EntryMigration{ + Header: "Migration header 2", + Body: "Migration body 2", + }, + }, + { + Description: "Changelog entry description 0", + Kind: Deprecation, + Breaking: false, + }, + { + Description: "Changelog entry description 0", + Kind: Bugfix, + Breaking: false, + }, + }, + mg: MigrationGuide{ + Version: "v999.999.999", + Weight: 1, + Migrations: []Migration{ + { + Header: "Migration header 0", + Body: "Migration body 0", + PullRequestLink: "[#999999](https://example.com/test/changelog/pulls/999999)", + }, + { + Header: "Migration header 1", + Body: "Migration body 1", + }, + { + Header: "Migration header 2", + Body: "Migration body 2", + }, + }, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + mg := MigrationGuideFromEntries(tc.version, tc.entries) + assert.Equal(t, tc.mg, mg) + }) + } +} diff --git a/hack/generate/changelog/util/testdata/ignore/00-template.yaml b/hack/generate/changelog/util/testdata/ignore/00-template.yaml new file mode 100644 index 00000000000..44f68b5ba65 --- /dev/null +++ b/hack/generate/changelog/util/testdata/ignore/00-template.yaml @@ -0,0 +1,36 @@ +# entries is a list of entries to include in +# release notes and/or the migration guide +entries: + - description: > + Description is the line that shows up in the CHANGELOG. This + should be formatted as markdown and be on a single line. Using + the YAML string '>' operator means you can write your entry + multiple lines and it will still be parsed as a single line. + + # kind is one of: + # - addition + # - change + # - deprecation + # - removal + # - bugfix + kind: "" + + # Is this a breaking change? + breaking: false + + # NOTE: ONLY USE `pull_request_override` WHEN ADDING THIS + # FILE FOR A PREVIOUSLY MERGED PULL_REQUEST! + # + # The generator auto-detects the PR number from the commit + # message in which this file was originally added. + # + # What is the pull request number (without the "#")? + # pull_request_override: 0 + + + # Migration can be defined to automatically add a section to + # the migration guide. This is required for breaking changes. + migration: + header: Header text for the migration section + body: > + Body of the migration section. diff --git a/hack/generate/changelog/util/testdata/ignore/more-fragments/ignored.yaml b/hack/generate/changelog/util/testdata/ignore/more-fragments/ignored.yaml new file mode 100644 index 00000000000..ec264e46281 --- /dev/null +++ b/hack/generate/changelog/util/testdata/ignore/more-fragments/ignored.yaml @@ -0,0 +1,19 @@ +entries: + - addition: Addition description + kind: addition + breaking: false + - description: Change description + kind: change + breaking: false + - description: Removal description + kind: removal + breaking: true + migration: + header: Header for removal migration + body: Body for removal migration + - description: Deprecation description + kind: deprecation + breaking: false + - description: Bugfix description + kind: bugfix + breaking: false \ No newline at end of file diff --git a/hack/generate/changelog/util/testdata/ignore/non-yaml.txt b/hack/generate/changelog/util/testdata/ignore/non-yaml.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/hack/generate/changelog/util/testdata/invalid_entry/fragment1.yaml b/hack/generate/changelog/util/testdata/invalid_entry/fragment1.yaml new file mode 100644 index 00000000000..4fe900e556a --- /dev/null +++ b/hack/generate/changelog/util/testdata/invalid_entry/fragment1.yaml @@ -0,0 +1,4 @@ +entries: + - description: Addition description 0 + kind: addition + breaking: true \ No newline at end of file diff --git a/hack/generate/changelog/util/testdata/invalid_yaml/fragment1.yaml b/hack/generate/changelog/util/testdata/invalid_yaml/fragment1.yaml new file mode 100644 index 00000000000..20d2807958b --- /dev/null +++ b/hack/generate/changelog/util/testdata/invalid_yaml/fragment1.yaml @@ -0,0 +1,18 @@ +- description: Addition description 0 + kind: addition + breaking: false +- description: Change description 0 + kind: change + breaking: false +- description: Removal description 0 + kind: removal + breaking: true + migration: + header: Header for removal migration 0 + body: Body for removal migration 0 +- description: Deprecation description 0 + kind: deprecation + breaking: false +- description: Bugfix description 0 + kind: bugfix + breaking: false \ No newline at end of file diff --git a/hack/generate/changelog/util/testdata/valid/fragment1.yaml b/hack/generate/changelog/util/testdata/valid/fragment1.yaml new file mode 100644 index 00000000000..4c8d7428153 --- /dev/null +++ b/hack/generate/changelog/util/testdata/valid/fragment1.yaml @@ -0,0 +1,19 @@ +entries: + - description: Addition description 0 + kind: addition + breaking: false + - description: Change description 0 + kind: change + breaking: false + - description: Removal description 0 + kind: removal + breaking: true + migration: + header: Header for removal migration 0 + body: Body for removal migration 0 + - description: Deprecation description 0 + kind: deprecation + breaking: false + - description: Bugfix description 0 + kind: bugfix + breaking: false \ No newline at end of file diff --git a/hack/generate/changelog/util/testdata/valid/fragment2.yaml b/hack/generate/changelog/util/testdata/valid/fragment2.yaml new file mode 100644 index 00000000000..00e1313a818 --- /dev/null +++ b/hack/generate/changelog/util/testdata/valid/fragment2.yaml @@ -0,0 +1,24 @@ +entries: + - description: Addition description 1 + kind: addition + breaking: false + pull_request_override: 999999 + - description: Change description 1 + kind: change + breaking: false + pull_request_override: 999999 + - description: Removal description 1 + kind: removal + breaking: true + pull_request_override: 999999 + migration: + header: Header for removal migration 1 + body: Body for removal migration 1 + - description: Deprecation description 1 + kind: deprecation + breaking: false + pull_request_override: 999999 + - description: Bugfix description 1 + kind: bugfix + breaking: false + pull_request_override: 999999 From 90e8c9781bf7b2cebfb5f8ca4ea38299a430d7bf Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 13 Apr 2020 21:23:54 -0400 Subject: [PATCH 18/21] fix sanity test checks --- hack/generate/changelog/gen-changelog.go | 2 +- hack/generate/changelog/util/changelog_test.go | 9 +++++++-- .../generate/changelog/util/migration_guide_test.go | 13 ------------- 3 files changed, 8 insertions(+), 16 deletions(-) diff --git a/hack/generate/changelog/gen-changelog.go b/hack/generate/changelog/gen-changelog.go index c54a85b2617..bf6ecb01d4b 100644 --- a/hack/generate/changelog/gen-changelog.go +++ b/hack/generate/changelog/gen-changelog.go @@ -45,7 +45,7 @@ func main() { log.Fatalf("failed to load fragments: %v", err) } if len(entries) == 0 { - log.Fatalf("no entries found") + log.Warnf("no entries found") } if validateOnly { diff --git a/hack/generate/changelog/util/changelog_test.go b/hack/generate/changelog/util/changelog_test.go index ab8c6a7364b..a07c45f17ab 100644 --- a/hack/generate/changelog/util/changelog_test.go +++ b/hack/generate/changelog/util/changelog_test.go @@ -249,8 +249,13 @@ func TestChangelog_Template(t *testing.T) { changelog: Changelog{ Version: "v999.999.999", Additions: []ChangelogEntry{ - {Description: "Changelog entry description 0."}, - {Description: "Changelog entry description 1.", Link: "[#999999](https://example.com/test/changelog/pulls/999999)"}, + { + Description: "Changelog entry description 0.", + }, + { + Description: "Changelog entry description 1.", + Link: "[#999999](https://example.com/test/changelog/pulls/999999)", + }, }, }, output: `## v999.999.999 diff --git a/hack/generate/changelog/util/migration_guide_test.go b/hack/generate/changelog/util/migration_guide_test.go index d7db60fe523..995bacdf6a8 100644 --- a/hack/generate/changelog/util/migration_guide_test.go +++ b/hack/generate/changelog/util/migration_guide_test.go @@ -1,7 +1,6 @@ package util import ( - "fmt" "io/ioutil" "os" "testing" @@ -11,18 +10,6 @@ import ( "github.com/stretchr/testify/assert" ) -func getMigrations(n int) []Migration { - sections := make([]Migration, n) - for i := 0; i < n; i++ { - sections[i] = Migration{ - Header: fmt.Sprintf("Migration header %d", i), - Body: fmt.Sprintf("Migration body %d", i), - PullRequestLink: "[#999999](https://example.com/test/changelog/pull/999999)", - } - } - return sections -} - func TestMigrationGuide_Template(t *testing.T) { testCases := []struct { name string From fa9f9f8497d772cf5c0cb39fcf149d26c86d14d0 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 13 Apr 2020 21:26:30 -0400 Subject: [PATCH 19/21] Makefile: include ./hack in unit tests --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 73bae8a7377..ad041b48ebc 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,7 @@ K8S_VERSION = v1.17.2 REPO = github.com/operator-framework/operator-sdk BUILD_PATH = $(REPO)/cmd/operator-sdk PKGS = $(shell go list ./... | grep -v /vendor/) -TEST_PKGS = $(shell go list ./... | grep -v -E 'github.com/operator-framework/operator-sdk/(hack/|test/)') +TEST_PKGS = $(shell go list ./... | grep -v -E 'github.com/operator-framework/operator-sdk/test/') SOURCES = $(shell find . -name '*.go' -not -path "*/vendor/*") ANSIBLE_BASE_IMAGE = quay.io/operator-framework/ansible-operator From afb26e61b056f67c449ca1adb7a94514475b3195 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 13 Apr 2020 23:35:42 -0400 Subject: [PATCH 20/21] goimports picked the wrong log package :) --- go.mod | 1 - go.sum | 1 - hack/generate/changelog/util/fragment.go | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/go.mod b/go.mod index bc9bea4808f..86aee7be57f 100644 --- a/go.mod +++ b/go.mod @@ -27,7 +27,6 @@ require ( github.com/pborman/uuid v1.2.0 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.5.1 - github.com/prometheus/common v0.9.1 github.com/rogpeppe/go-internal v1.5.0 github.com/rubenv/sql-migrate v0.0.0-20191025130928-9355dd04f4b3 // indirect github.com/sergi/go-diff v1.0.0 diff --git a/go.sum b/go.sum index 8ef1ea05cac..c061e058ffe 100644 --- a/go.sum +++ b/go.sum @@ -99,7 +99,6 @@ github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuy github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4 h1:Hs82Z41s6SdL1CELW+XaDYmOH4hkBN4/N9og/AsOv7E= github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0= -github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d h1:UQZhZ2O0vMHr2cI+DC1Mbh0TJxzA3RcLoMsFw+aXw7E= github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho= github.com/aliyun/aliyun-oss-go-sdk v2.0.4+incompatible/go.mod h1:T/Aws4fEfogEE9v+HPhhw+CntffsBHJ8nXQCwKr0/g8= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= diff --git a/hack/generate/changelog/util/fragment.go b/hack/generate/changelog/util/fragment.go index 035d0b59888..4a8b28980fe 100644 --- a/hack/generate/changelog/util/fragment.go +++ b/hack/generate/changelog/util/fragment.go @@ -10,7 +10,7 @@ import ( "strconv" "strings" - "github.com/prometheus/common/log" + log "github.com/sirupsen/logrus" "gopkg.in/yaml.v2" ) From a7d79dca354024ff1f3b3963d743205ce7285172 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 13 Apr 2020 23:38:44 -0400 Subject: [PATCH 21/21] fix order of imports --- hack/generate/changelog/gen-changelog.go | 4 ++-- hack/generate/changelog/util/changelog_test.go | 1 - hack/generate/changelog/util/migration_guide_test.go | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/hack/generate/changelog/gen-changelog.go b/hack/generate/changelog/gen-changelog.go index bf6ecb01d4b..fcdf4de705c 100644 --- a/hack/generate/changelog/gen-changelog.go +++ b/hack/generate/changelog/gen-changelog.go @@ -6,10 +6,10 @@ import ( "path/filepath" "strings" - "github.com/operator-framework/operator-sdk/hack/generate/changelog/util" - "github.com/blang/semver" log "github.com/sirupsen/logrus" + + "github.com/operator-framework/operator-sdk/hack/generate/changelog/util" ) const repo = "github.com/operator-framework/operator-sdk" diff --git a/hack/generate/changelog/util/changelog_test.go b/hack/generate/changelog/util/changelog_test.go index a07c45f17ab..a5d261a7272 100644 --- a/hack/generate/changelog/util/changelog_test.go +++ b/hack/generate/changelog/util/changelog_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/blang/semver" - "github.com/stretchr/testify/assert" ) diff --git a/hack/generate/changelog/util/migration_guide_test.go b/hack/generate/changelog/util/migration_guide_test.go index 995bacdf6a8..3f08b666206 100644 --- a/hack/generate/changelog/util/migration_guide_test.go +++ b/hack/generate/changelog/util/migration_guide_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/blang/semver" - "github.com/stretchr/testify/assert" )