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

Update export_dashboards.go #7101

Merged
merged 4 commits into from
May 16, 2018

Conversation

hakan-carlsson
Copy link
Contributor

Added support to include index-pattern in output JSON. Also added -quiet option to make it silent.

Added support to include index-pattern in output JSON. Also added -quiet option to make it silent.
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@@ -113,11 +113,16 @@ func ReadManifest(file string) ([]map[string]string, error) {
return manifest.Dashboards, nil
}

var indexPattern bool = false
var quiet bool = false

Choose a reason for hiding this comment

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

should omit type bool from declaration of var quiet; it will be inferred from the right-hand side

@@ -113,11 +113,16 @@ func ReadManifest(file string) ([]map[string]string, error) {
return manifest.Dashboards, nil
}

var indexPattern bool = false

Choose a reason for hiding this comment

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

should omit type bool from declaration of var indexPattern; it will be inferred from the right-hand side

@@ -48,7 +48,7 @@ func ExtractIndexPattern(body []byte) ([]byte, error) {
if !ok {
return nil, fmt.Errorf("type key not found or not string")
}
if _type != "index-pattern" {
if _type != "index-pattern" || indexPattern == true {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not write || indexPattern instead of || indexPattern == true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, works too. IMHO I like to be explicit.

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked other parts of the code. Although, I find it weird, other parts of the code does include bool checking using == true, so I am ok with letting this go.

@@ -68,7 +68,7 @@ func Export(client *http.Client, conn string, dashboard string, out string) erro
params.Add("dashboard", dashboard)

fullURL := makeURL(conn, exportAPI, params)
fmt.Printf("Calling HTTP GET %v\n", fullURL)
if ! quiet { fmt.Printf("Calling HTTP GET %v\n", fullURL) }
Copy link
Contributor

@kvch kvch May 16, 2018

Choose a reason for hiding this comment

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

Could you please add new lines after { and fullURL)? Please also remove the space between ! and quiet.

if !quiet {
    fmt.Printf("Calling HTTP GET %v\n", fullURL)
}

Most of the codebase does not use this format. So in order to keep the uniformity of the code, please change it.

@@ -95,7 +95,7 @@ func Export(client *http.Client, conn string, dashboard string, out string) erro

err = ioutil.WriteFile(out, body, 0666)

fmt.Printf("The dashboard %s was exported under the %s file\n", dashboard, out)
if ! quiet { fmt.Printf("The dashboard %s was exported under the %s file\n", dashboard, out) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here with the newlines.

if !quiet {
    fmt.Printf("The dashboard %s was exported under the %s file\n", dashboard, out)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, point taken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made changes to indents, removed explicit =='s and removed explict bool

@kvch
Copy link
Contributor

kvch commented May 16, 2018

jenkins test this

changes after review inputs
Copy link
Contributor

@kvch kvch left a comment

Choose a reason for hiding this comment

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

Great! Thanks for the contribution. After the CIs turn green, it can be merged.

@kvch
Copy link
Contributor

kvch commented May 16, 2018

jenkins. test this, please

@kvch
Copy link
Contributor

kvch commented May 16, 2018

Could you please add your change to CHANGELOG.asciidoc?

@kvch
Copy link
Contributor

kvch commented May 16, 2018

Thanks.

@kvch kvch merged commit 175d3e3 into elastic:master May 16, 2018
stevea78 pushed a commit to stevea78/beats that referenced this pull request May 20, 2018
* Update export_dashboards.go

Added support to include index-pattern in output JSON. Also added -quiet option to make it silent.

* Update export_dashboards.go

changes after review inputs

* Update CHANGELOG.asciidoc
stevea78 pushed a commit to stevea78/beats that referenced this pull request May 20, 2018
* Update export_dashboards.go

Added support to include index-pattern in output JSON. Also added -quiet option to make it silent.

* Update export_dashboards.go

changes after review inputs

* Update CHANGELOG.asciidoc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants