add functions to encode/decode base64 strings with specific encoding #25470
Conversation
|
Thanks for the pr @r0bnet :) Just a quick note that we're not going to be merging external PRs for new functionality (or much else, short of bugs) until 0.13 is released. We'll revisit this following Terraform's 0.13.0 release. |
|
Hey @pkolyvas, that's what I expected but not what I hoped ;) I'll try to keep it updated once in a while with a rebase so that merging later will be faster hopefully. Thanks. |
Codecov Report
|
|
@apparentlymart is it possible that you or someone else could check this PR? Just rebased it and back then it was blocked due to pre-0.13 |
apparentlymart
force-pushed
the
text-encode-functions
branch
from
1a8d779 to
ba6ab4a
Compare
These were initially introduced as functions with "encode" and "decode" prefixes, but that doesn't match with our existing convention of putting the encoding format first so that the encode and decode functions will group together in a alphabetically-ordered function list. "text" is not really a defined serialization format, but it's a short word that hopefully represents well enough what these functions are aiming to encode and decode, while being consistent with existing functions like jsonencode/jsondecode, yamlencode/yamldecode, etc. The "base64" at the end here is less convincing because there is precedent for that modifier to appear both at the beginning and the end in our existing function names. I chose to put it at the end here because that seems to be our emergent convention for situations where the base64 encoding is a sort of secondary modifier alongside the primary purpose of the function, as we see with "filebase64". (base64gzip is an exception here, but it seems outvoted by the others.)
apparentlymart
force-pushed
the
text-encode-functions
branch
from
ba6ab4a to
1dc4950
Compare
|
Hi @r0bnet! Sorry for the delay in looking at this... my work for Terraform 0.14.0 ran on a little longer than I'd hoped, so I'm only just getting to PR reviewing now. What you wrote here is great, and I'm grateful that you took the time to contribute it. I made some small tweaks to the error messages, with the main functional change being that it will now use After seeing this all in context I realized also that the function names I'd suggested to you in #25387 were inconsistent with our existing naming scheme of putting the "encoding type" at the start of the name, so that the encode/decode pairs of functions can sort together in an alphabetical list of functions. With that in mind, I also added a follow-up commit here to rename them to With all of that said, I'm going to merge this now. I had also intended to merge this during the v0.14.0 development window, and so as a special exception I'm going to backport it into the v0.14 branch even though typically at this stage we'd normally be primarily merging only bug fix changes based on the beta feedback. That means these functions will be available for the first time in v0.14.0-beta2, and should in turn therefore be included in the final v0.14.0 release in a few weeks. Thanks again, and sorry for the delay! |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked as resolved and limited conversation to collaborators
Hey,
my first PR here. Hope I didn't miss anything.
Background of this PR is:
#25387