These comments were a reasonable attempt at ensuring the documentation
matched reality, but the checkbox in the pull request template is much
more likely to produce results.
As I skimmed through that section I noticed it wasn't immediately clear
whether a line of text was referring to the example above it or the one
below it.
It seems like this needs more information, like why you'd want to put
something in one stanza or the other, but I don't really know enough
about drone to give useful advice.
Note that as of this writing, docs/parameter_reference.md (and required-
params documentation in README.md) doesn't actually exist; I created it
in ef66bc0 but that commit hasn't been merged yet.
It's a little tricky to find a balance between "brittle" and "thorough"
in this test--I'd like to verify that e.g. the certificate is in
clusters[0].cluster.certificate-authority-data, not at the root. On the
other hand, we can't actually show that it's a valid kubeconfig file
without actually *using* it, so there's a hard upper limit on the
strength of the assertions. I've settled on verifying that all the
settings make it into the file and the file is syntactically-valid yaml.
I was unhappy with the comments-in-yaml approach; it required
duplicating a lot of information and it was hard to find a balance
between "usefully thorough" and "readably concise.""
Redacting KubeToken may not be sufficient, since it's possible that
someone would put secrets in Values or StringValues. Unilaterally
redacting those seems unhelpful, though, since they may be the very
thing the user is trying to debug. I've settled on redacting the obvious
field without trying to promise that all sensitive data will be hidden.
I'd like to keep Prefix's scope fairly limited, because it has potential
to spiral into something magnificently complex. You get one prefix
setting, it goes in `settings` not `environment`, end of feature.
Trying to guess in advance which part of the config a user will put in
the `settings` section and which they'll put in `environment` is a
fool's errand. Just let everything go in either place.
The ServiceAccount field only had an `envconfig` tag (as opposed to
`split_words`) because that triggered envconfig to look for the non-
prefixed form. Now that we're finding non-prefixed forms of everything,
we can use the clearer/more concise tag.
Note that TestPopulateWithConflictingVariables isn't meant to say
"here's what behavior we *want*" so much as "here's what the behavior
*is*." I don't think one thing is any better than the other, but we
should know which one we're getting.
Erin Call
Joachim Hill-Grannec