Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use string builder to avoid copies on notes rendering #978

Merged
merged 1 commit into from
Dec 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions cmd/release-notes/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,12 +294,15 @@ func WriteReleaseNotes(releaseNotes notes.ReleaseNotes, history notes.ReleaseNot
return errors.Wrapf(err, "creating release note document")
}

if err := notes.RenderMarkdown(
output, doc, opts.ReleaseBucket,
opts.ReleaseTars, opts.StartRev, opts.EndRev,
); err != nil {
markdown, err := notes.RenderMarkdown(
doc, opts.ReleaseBucket, opts.ReleaseTars, opts.StartRev, opts.EndRev,
)
if err != nil {
return errors.Wrapf(err, "rendering release note document to markdown")
}
if _, err := output.WriteString(markdown); err != nil {
return errors.Wrapf(err, "writing output file")
}

default:
return errors.Errorf("%q is an unsupported format", opts.Format)
Expand Down
83 changes: 45 additions & 38 deletions pkg/notes/document.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,10 @@ func CreateDocument(notes ReleaseNotes, history ReleaseNotesHistory) (*Document,

// RenderMarkdown accepts a Document and writes a version of that document to
// supplied io.Writer in markdown format.
func RenderMarkdown(w io.Writer, doc *Document, bucket, tars, prevTag, newTag string) error {
if err := createDownloadsTable(w, bucket, tars, prevTag, newTag); err != nil {
return err
func RenderMarkdown(doc *Document, bucket, tars, prevTag, newTag string) (string, error) {
o := &strings.Builder{}
if err := createDownloadsTable(o, bucket, tars, prevTag, newTag); err != nil {
return "", err
}

// we always want to render the document with SIGs in alphabetical order
Expand All @@ -123,103 +124,109 @@ func RenderMarkdown(w io.Writer, doc *Document, bucket, tars, prevTag, newTag st
}
sort.Strings(sortedSIGs)

// this is a helper so that we don't have to check err != nil on every write

// first, we create a long-lived err that we can re-use
var err error

// write is a helper that writes a string to the in-scope io.Writer w
write := func(s string) {
// if write has already failed, just return and don't do anything
if err != nil {
return
}
// perform the write
_, err = w.Write([]byte(s))
nl := func() {
o.WriteRune('\n')
}
nlnl := func() {
nl()
nl()
}

// writeNote encapsulates the pre-processing that might happen on a note text
// before it gets bulleted and written to the io.Writer
writeNote := func(s string) {
if !strings.HasPrefix(s, "- ") {
s = "- " + s
const prefix = "- "
if !strings.HasPrefix(s, prefix) {
o.WriteString(prefix)
}
write(s + "\n")
o.WriteString(s)
nl()
}

// the "Action Required" section
if len(doc.ActionRequired) > 0 {
write("## Action Required\n\n")
o.WriteString("## Action Required")
nlnl()
for _, note := range doc.ActionRequired {
writeNote(note)
}
write("\n\n")
nlnl()
}

// the "New Feautres" section
if len(doc.NewFeatures) > 0 {
write("## New Features\n\n")
o.WriteString("## New Features")
nlnl()
for _, note := range doc.NewFeatures {
writeNote(note)
}
write("\n\n")
nlnl()
}

// the "API Changes" section
if len(doc.APIChanges) > 0 {
write("### API Changes\n\n")
o.WriteString("### API Changes")
nlnl()
for _, note := range doc.APIChanges {
writeNote(note)
}
write("\n\n")
nlnl()
}

// the "Duplicate Notes" section
if len(doc.Duplicates) > 0 {
write("### Notes from Multiple SIGs\n\n")
o.WriteString("### Notes from Multiple SIGs")
nlnl()
for header, notes := range doc.Duplicates {
write(fmt.Sprintf("#### %s\n\n", header))
o.WriteString("#### ")
o.WriteString(header)
nlnl()
for _, note := range notes {
writeNote(note)
}
write("\n")
nl()
}
write("\n")
nl()
}

// each SIG gets a section (in alphabetical order)
if len(sortedSIGs) > 0 {
write("### Notes from Individual SIGs\n\n")
o.WriteString("### Notes from Individual SIGs")
nlnl()
for _, sig := range sortedSIGs {
write("#### SIG " + prettySIG(sig) + "\n\n")
o.WriteString("#### SIG ")
o.WriteString(prettySIG(sig))
nlnl()
for _, note := range doc.SIGs[sig] {
writeNote(note)
}
write("\n")
nl()
}
write("\n\n")
nlnl()
}

// the "Bug Fixes" section
if len(doc.BugFixes) > 0 {
write("### Bug Fixes\n\n")
o.WriteString("### Bug Fixes")
nlnl()
for _, note := range doc.BugFixes {
writeNote(note)
}
write("\n\n")
nlnl()
}

// we call the uncategorized notes "Other Notable Changes". ideally these
// notes would at least have a SIG label.
if len(doc.Uncategorized) > 0 {
write("### Other Notable Changes\n\n")
o.WriteString("### Other Notable Changes")
nlnl()
for _, note := range doc.Uncategorized {
writeNote(note)
}
write("\n\n")
nlnl()
}

return err
return o.String(), nil
}

// prettySIG takes a sig name as parsed by the `sig-foo` label and returns a
Expand Down
9 changes: 5 additions & 4 deletions pkg/notes/document_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ limitations under the License.
package notes

import (
"bytes"
"io/ioutil"
"os"
"path/filepath"
"strings"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -44,7 +44,6 @@ func TestPrettySIG(t *testing.T) {

func TestCreateDownloadsTable(t *testing.T) {
// Given
output := &bytes.Buffer{}
dir, err := ioutil.TempDir("", "")
require.Nil(t, err)
defer os.RemoveAll(dir)
Expand Down Expand Up @@ -80,8 +79,10 @@ func TestCreateDownloadsTable(t *testing.T) {
}

// When
require.Nil(t, createDownloadsTable(output, "kubernetes-release", dir,
"v1.16.0", "v1.16.1"))
output := &strings.Builder{}
require.Nil(t, createDownloadsTable(
output, "kubernetes-release", dir, "v1.16.0", "v1.16.1",
))

// Then
require.Equal(t, output.String(), `# v1.16.1
Expand Down