Skip to content
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
12a17ea
Check and generate multiple README.md files in one package
Mar 9, 2021
a3886c0
Merge branch 'master' into additional_readmes
ycombinator Mar 9, 2021
ae666b8
move ReadDir into IsReadmeUpToDate and UpdateReadme
Mar 10, 2021
0981abe
Merge branch 'master' of github.com:elastic/elastic-package into addi…
Mar 10, 2021
d57e8ba
Merge branch 'additional_readmes' of github.com:kaiyan-sheng/elastic-…
Mar 10, 2021
7ef4eb5
Merge branch 'master' into additional_readmes
ycombinator Mar 10, 2021
3e762da
Merge branch 'master' into additional_readmes
Mar 12, 2021
303bb12
Create AreReadmesUpToDate and UpdateReadmes functions
Mar 13, 2021
17afb55
print elastic-package build output in alphabetical order
Mar 13, 2021
4f603f8
list all file names that need to be updated
Mar 13, 2021
5898df2
make _dev/build/docs not required
Mar 15, 2021
6cf1ce4
Merge branch 'master' into additional_readmes
mtojek Mar 15, 2021
e5983ce
check os.IsNotExist and use fileName instead
Mar 16, 2021
380019c
Merge branch 'additional_readmes' of github.com:kaiyan-sheng/elastic-…
Mar 16, 2021
41c2ff2
fix error message
Mar 16, 2021
9d9039c
Merge branch 'master' into additional_readmes
mtojek Mar 16, 2021
9a85bb5
use cmd.Printf for error messages
Mar 18, 2021
da89e55
Merge branch 'additional_readmes' of github.com:kaiyan-sheng/elastic-…
Mar 18, 2021
cb3e976
fix error messages
Mar 18, 2021
9ff5e57
add ignore_missing
Mar 18, 2021
1673749
rerun elastic-package test pipeline for aws
Mar 18, 2021
d24311c
test with adding package level vars
Mar 22, 2021
17506c5
check input is nil before merging vars
Mar 23, 2021
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
16 changes: 11 additions & 5 deletions cmd/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package cmd

import (
"strings"

"github.com/pkg/errors"
"github.com/spf13/cobra"

Expand Down Expand Up @@ -36,15 +38,19 @@ func setupBuildCommand() *cobra.Command {
func buildCommandAction(cmd *cobra.Command, args []string) error {
cmd.Println("Build the package")

target, err := docs.UpdateReadme()
targets, err := docs.UpdateReadmes()
if err != nil {
return errors.Wrapf(err, "updating %s file failed", docs.ReadmeFile)
return errors.Wrap(err, "updating files failed")
}
if target != "" {
cmd.Printf("%s file rendered: %s\n", docs.ReadmeFile, target)

for _, target := range targets {
if target != "" {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the case that the target is empty?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be empty when README file is static and can't be generated from the template file. This came from generateReadme(fileName, packageRoot) function.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see! It might be cleaner to skip this file on the lower layer (in the docs.UpdateReadmes()), so "" (empty string) won't leak here.

splitTarget := strings.Split(target, "/")
cmd.Printf("%s file rendered: %s\n", splitTarget[len(splitTarget)-1], target)
}
}

target, err = builder.BuildPackage()
target, err := builder.BuildPackage()
if err != nil {
return errors.Wrap(err, "building package failed")
}
Expand Down
25 changes: 20 additions & 5 deletions cmd/lint.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package cmd

import (
"fmt"

"github.com/pkg/errors"
"github.com/spf13/cobra"

Expand Down Expand Up @@ -44,12 +43,28 @@ func lintCommandAction(cmd *cobra.Command, args []string) error {
return errors.Wrap(err, "locating package root failed")
}

ok, err := docs.IsReadmeUpToDate()
readmeFiles, err := docs.AreReadmesUpToDate()
if err != nil {
return errors.Wrapf(err, "can't check if %s file is up-to-date", docs.ReadmeFile)
return errors.Wrap(err, "checking readme files are up-to-date failed")
}

outdatedStr := ""
errStr := ""
for _, f := range readmeFiles {
if !f.UpToDate {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to capture all outdated file names here to display in the error message. It's not pretty again. Is there a better way to do so?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the other comment I suggested to return an error if something bad happens.

You can check if docs.AreReadmesUpToDate() returned an error and any readmeFiles. If so, you can print the error and conditionally iterate over readmeFiles. No need to concatenate single error message. You can return a generic error and use cmd.Printf to iterate over problematic files. We used to mix cmd.Printf and errrors.Wrap for such cases, you can take a look at cmd/build.go and internal/builder/packages.go.

outdatedStr += f.FileName + " "
}
if f.Error != nil {
errStr += f.FileName + " "
}
}
if !ok {
return fmt.Errorf("%s file is outdated. Rebuild the package with 'elastic-package build'", docs.ReadmeFile)

if outdatedStr != "" {
return fmt.Errorf("%s are outdated. Rebuild the package with 'elastic-package build'", outdatedStr)
}

if errStr != "" {
return errors.Wrapf(err, "check if %s are up-to-date failed", errStr)
}

err = validator.ValidateFromPath(packageRootPath)
Expand Down
125 changes: 89 additions & 36 deletions internal/docs/readme.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,56 @@ import (
"github.com/elastic/elastic-package/internal/packages"
)

// ReadmeFile for the Elastic package
const ReadmeFile = "README.md"
// ReadmeFile contains file name and status of each readme file.
type ReadmeFile struct {
Comment thread
kaiyan-sheng marked this conversation as resolved.
FileName string
UpToDate bool
Error error
}

// AreReadmesUpToDate function checks if all the .md readme file are up-to-date.
func AreReadmesUpToDate() ([]ReadmeFile, error) {
packageRoot, err := packages.MustFindPackageRoot()
if err != nil {
return nil, errors.Wrap(err, "package root not found")
}

files, err := ioutil.ReadDir(filepath.Join(packageRoot, "_dev", "build", "docs"))
if err != nil && !os.IsNotExist(err) {
return nil, errors.Wrap(err, "reading directory entries failed")
}

var readmeFiles []ReadmeFile
for _, f := range files {
fileName := f.Name()
ok, err := isReadmeUpToDate(fileName, packageRoot)
readmeFile := ReadmeFile{
FileName: fileName,
UpToDate: ok,
Error: err,
}
readmeFiles = append(readmeFiles, readmeFile)
}
return readmeFiles, nil
Comment thread
kaiyan-sheng marked this conversation as resolved.
}

// IsReadmeUpToDate function checks if the README file is up-to-date.
func IsReadmeUpToDate() (bool, error) {
logger.Debugf("Check if %s is up-to-date", ReadmeFile)
func isReadmeUpToDate(fileName, packageRoot string) (bool, error) {
logger.Debugf("Check if %s is up-to-date", fileName)

packageRoot, err := packages.MustFindPackageRoot()
if err != nil {
return false, errors.Wrap(err, "package root not found")
}

rendered, shouldBeRendered, err := generateReadme(packageRoot)
rendered, shouldBeRendered, err := generateReadme(fileName, packageRoot)
if err != nil {
return false, err
return false, errors.Wrap(err, "generate readme file failed")
Comment thread
kaiyan-sheng marked this conversation as resolved.
Outdated
}
if !shouldBeRendered {
return true, nil // README file is static and doesn't use template.
}

existing, found, err := readReadme(packageRoot)
existing, found, err := readReadme(fileName, packageRoot)
if err != nil {
return false, errors.Wrap(err, "reading README file failed")
}
Expand All @@ -47,66 +76,90 @@ func IsReadmeUpToDate() (bool, error) {
return bytes.Equal(existing, rendered), nil
}

// UpdateReadme function updates the README file using ą defined template file. The function doesn't perform any action
// if the template file is not present.
func UpdateReadme() (string, error) {
logger.Debugf("Update the %s file", ReadmeFile)
// UpdateReadmes function updates all .md readme files using a defined template
// files. The function doesn't perform any action if the template file is not present.
func UpdateReadmes() ([]string, error) {
packageRoot, err := packages.MustFindPackageRoot()
if err != nil {
return nil, errors.Wrap(err, "package root not found")
}

readmeFiles, err := ioutil.ReadDir(filepath.Join(packageRoot, "_dev", "build", "docs"))
Comment thread
mtojek marked this conversation as resolved.
if err != nil && !os.IsNotExist(err) {
return nil, errors.Wrap(err, "reading directory entries failed")
}

var targets []string
for _, readme := range readmeFiles {
fileName := readme.Name()
target, err := updateReadme(fileName, packageRoot)
if err != nil {
return nil, errors.Wrapf(err, "update readme file %s failed", fileName)
Comment thread
kaiyan-sheng marked this conversation as resolved.
Outdated
}

targets = append(targets, target)
}
return targets, nil
}

func updateReadme(fileName, packageRoot string) (string, error) {
logger.Debugf("Update the %s file", fileName)

packageRoot, err := packages.MustFindPackageRoot()
if err != nil {
return "", errors.Wrap(err, "package root not found")
}

rendered, shouldBeRendered, err := generateReadme(packageRoot)
rendered, shouldBeRendered, err := generateReadme(fileName, packageRoot)
if err != nil {
return "", err
}
if !shouldBeRendered {
return "", nil
}

target, err := writeReadme(packageRoot, rendered)
target, err := writeReadme(fileName, packageRoot, rendered)
if err != nil {
return "", errors.Wrapf(err, "writing %s file failed", ReadmeFile)
return "", errors.Wrapf(err, "writing %s file failed", fileName)
}
return target, nil
}

func generateReadme(packageRoot string) ([]byte, bool, error) {
logger.Debugf("Generate %s file (package: %s)", ReadmeFile, packageRoot)
templatePath, found, err := findReadmeTemplatePath(packageRoot)
func generateReadme(fileName, packageRoot string) ([]byte, bool, error) {
logger.Debugf("Generate %s file (package: %s)", fileName, packageRoot)
templatePath, found, err := findReadmeTemplatePath(fileName, packageRoot)
if err != nil {
return nil, false, errors.Wrapf(err, "can't locate %s template file", ReadmeFile)
return nil, false, errors.Wrapf(err, "can't locate %s template file", fileName)
}
if !found {
logger.Debug("README file is static, can't be generated from the template file")
return nil, false, nil
}
logger.Debugf("Template file for %s found: %s", ReadmeFile, templatePath)
logger.Debugf("Template file for %s found: %s", fileName, templatePath)

rendered, err := renderReadme(packageRoot, templatePath)
rendered, err := renderReadme(fileName, packageRoot, templatePath)
if err != nil {
return nil, true, errors.Wrap(err, "rendering Readme failed")
}
return rendered, true, nil
}

func findReadmeTemplatePath(packageRoot string) (string, bool, error) {
templatePath := filepath.Join(packageRoot, "_dev", "build", "docs", ReadmeFile)
func findReadmeTemplatePath(fileName, packageRoot string) (string, bool, error) {
templatePath := filepath.Join(packageRoot, "_dev", "build", "docs", fileName)
_, err := os.Stat(templatePath)
if err != nil && os.IsNotExist(err) {
return "", false, nil // README.md file not found
}
if err != nil {
return "", false, errors.Wrapf(err, "can't located the %s file", ReadmeFile)
return "", false, errors.Wrapf(err, "can't located the %s file", fileName)
Comment thread
kaiyan-sheng marked this conversation as resolved.
Outdated
}
return templatePath, true, nil
}

func renderReadme(packageRoot, templatePath string) ([]byte, error) {
logger.Debugf("Render %s file (package: %s, templatePath: %s)", ReadmeFile, packageRoot, templatePath)
func renderReadme(fileName, packageRoot, templatePath string) ([]byte, error) {
Comment thread
kaiyan-sheng marked this conversation as resolved.
logger.Debugf("Render %s file (package: %s, templatePath: %s)", fileName, packageRoot, templatePath)

t := template.New(ReadmeFile)
t := template.New(fileName)
t, err := t.Funcs(template.FuncMap{
"event": func(dataStreamName string) (string, error) {
return renderSampleEvent(packageRoot, dataStreamName)
Expand All @@ -127,10 +180,10 @@ func renderReadme(packageRoot, templatePath string) ([]byte, error) {
return rendered.Bytes(), nil
}

func readReadme(packageRoot string) ([]byte, bool, error) {
logger.Debugf("Read existing %s file (package: %s)", ReadmeFile, packageRoot)
func readReadme(fileName, packageRoot string) ([]byte, bool, error) {
logger.Debugf("Read existing %s file (package: %s)", fileName, packageRoot)

readmePath := filepath.Join(packageRoot, "docs", ReadmeFile)
readmePath := filepath.Join(packageRoot, "docs", fileName)
b, err := ioutil.ReadFile(readmePath)
if err != nil && os.IsNotExist(err) {
return nil, false, nil
Expand All @@ -141,8 +194,8 @@ func readReadme(packageRoot string) ([]byte, bool, error) {
return b, true, err
}

func writeReadme(packageRoot string, content []byte) (string, error) {
logger.Debugf("Write %s file (package: %s)", ReadmeFile, packageRoot)
func writeReadme(fileName, packageRoot string, content []byte) (string, error) {
logger.Debugf("Write %s file (package: %s)", fileName, packageRoot)

docsPath := docsPath(packageRoot)
logger.Debugf("Create directories: %s", docsPath)
Expand All @@ -151,8 +204,8 @@ func writeReadme(packageRoot string, content []byte) (string, error) {
return "", errors.Wrapf(err, "mkdir failed (path: %s)", docsPath)
}

aReadmePath := readmePath(packageRoot)
logger.Debugf("Write %s file to: %s", ReadmeFile, aReadmePath)
aReadmePath := readmePath(fileName, packageRoot)
logger.Debugf("Write %s file to: %s", fileName, aReadmePath)

err = ioutil.WriteFile(aReadmePath, content, 0644)
if err != nil {
Expand All @@ -161,8 +214,8 @@ func writeReadme(packageRoot string, content []byte) (string, error) {
return aReadmePath, nil
}

func readmePath(packageRoot string) string {
return filepath.Join(docsPath(packageRoot), ReadmeFile)
func readmePath(fileName, packageRoot string) string {
return filepath.Join(docsPath(packageRoot), fileName)
}

func docsPath(packageRoot string) string {
Expand Down
Loading