Skip to content
This repository was archived by the owner on Jan 21, 2020. It is now read-only.

Conversation

kaufers
Copy link
Contributor

@kaufers kaufers commented May 12, 2017

Instead of assuming that all CLI template --var values are String, this adds support to convert the String value to an int or boolean.

Closes #542

Signed-off-by: Steven Kaufer kaufer@us.ibm.com

@kaufers
Copy link
Contributor Author

kaufers commented May 12, 2017

@chungers initially I was using the infrakit/pkg/cli/context.go parseBool function that converted standard string like true, T, yes, Y, 0 to the true boolean. However, this seemed like it accepted too many values (since all ints were being converted to booleans and many strings like yes were as well); with that information I thought that simply using the golang reserved true/false for boolean would prevent unintentional string-to-boolean conversions.

@codecov
Copy link

codecov bot commented May 12, 2017

Codecov Report

Merging #543 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #543   +/-   ##
=======================================
  Coverage   56.66%   56.66%           
=======================================
  Files          57       57           
  Lines        3937     3937           
=======================================
  Hits         2231     2231           
  Misses       1420     1420           
  Partials      286      286

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12aaeda...d8efe70. Read the comment docs.

// that a string like "no" will remain a string.
if intVal, err := strconv.Atoi(val); err == nil {
engine.Global(key, intVal)
} else if val == "true" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see your point about not using the parseBool function. In this case, there's a strconv.ParseBool that should do the job.. https://golang.org/pkg/strconv/#ParseBool

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, will update to use that so that the parsing is consistent. Thanks for the review!

Instead of assuming that all CLI template `--var` values are String, this
adds support to convert the String value to an int or boolean.

Closes docker-archive#542

Signed-off-by: Steven Kaufer <kaufer@us.ibm.com>
@kaufers kaufers force-pushed the template-cli-var-bool branch from 7172b45 to d8efe70 Compare May 12, 2017 18:04
@chungers chungers merged commit 8f9f533 into docker-archive:master May 12, 2017
chungers pushed a commit to chungers/infrakit that referenced this pull request Sep 30, 2017
Added ability to handle Regions with 2 and 3 AZ's in one template.
chungers pushed a commit to chungers/infrakit that referenced this pull request Oct 1, 2017
Added ability to handle Regions with 2 and 3 AZ's in one template.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants