Introduce new 'random_integer' provider #12
Conversation
This provider allows the random generation of integer values in a given inclusive range.
bitbrain
changed the title
website/docs/r/range.html.md
Outdated
| min = 1 | ||
| max = 99999 | ||
| keepers = { | ||
| # Generate a new pet name each time we switch to a new AMI id |
There was a problem hiding this comment.
Thanks for working on this, @bitbrain!
This seems like a fine idea to me. My feedback below is just some small naming questions; in terms of functionality this looks great to me.
website/docs/r/range.html.md
Outdated
|
|
||
| The following attributes are supported: | ||
|
|
||
| * `id` - (int) The random range value. |
There was a problem hiding this comment.
It feels a little unintuitive to use id for this, since the result probably isn't an id in the usual sense we mean it.
We are required to call SetId to give Terraform something to track with, but I'd suggest that we consider that an undocumented implementation detail and instead publish a computed attribute called result which is documented here.
priority = "${random_range.priority.result}"What do you think? (I chose result here again for consistency with random_string)
There was a problem hiding this comment.
I tried to stick to the notation of already existing providers (e.g. pet provider) which also populates an id attribute as a result.
Regardless, I agree with you, result should be it! 👍
random/provider.go
Outdated
| @@ -15,6 +15,7 @@ func Provider() terraform.ResourceProvider { | |||
| "random_shuffle": resourceShuffle(), | |||
| "random_pet": resourcePet(), | |||
| "random_string": resourceString(), | |||
| "random_range": resourceRange(), | |||
There was a problem hiding this comment.
Naming things is always tricky... when I first saw this name I expected that the result would be a random start/end segment of a list, or some such thing, since "range" makes me think of a sub-portion of a list.
What do you think about calling this random_integer, as a sibling of random_string?
There was a problem hiding this comment.
I had a similar thought. However, random_integer gives me the impression that it does not necessarily require a range but allows me to get any integer. I could also make min and max optional and set it to min=0 and max=INTEGER_MAX. 🧐
|
@apparentlymart Updated! |
bitbrain
changed the title
|
Thanks, @bitbrain! |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
This provider allows the random generation of integer values in a given inclusive range. This Pull Request aims to close #4.
Feedback welcome. 🙈