Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Import functions from Terraform #37
Import functions from Terraform #37
Changes from 7 commits
571e23c
4029697
ab0a7c5
b3ee7c0
120ea08
76240aa
c9e60db
b92e91b
d0ca986
4b47e44
98aad05
811b229
8e00637
038b34f
28fcda2
446191d
5738b88
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is specifically using RFC3339; do you want me to make this one a
MakeTimeAddFunc(layout string)
to allow/force a user to pick a layout ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
formatdate
function that's already in here has created the precedent that RFC3339 is the canonical time format forcty
, so I think it makes sense to continue that precedent here. Users that need a different date format as output can pass the result toformatdate
to change it.This convention that timestamps are just strings written in RFC3339 notation actually originated in Terraform itself prior to
cty
existing, but I just embraced it here since it seemed like a good enough idea. I wanted to point that out because that is a Terraform-wide convention: provider plugins are expected to produce and accept timestamps in RFC3339 format, even if that means that the provider has to do conversions itself from whatever representation a remote API is using.Hopefully Packer would be able to dictate a similar dictum so that everything will compose together well, but I don't know if there's existing precedent in Packer for timestamps in other formats. If so, Packer might end up wanting its own time-manipulation functions in order to work well with the existing conventions. I'd prefer to keep the
cty
convention simple either way, and let individual applications handle their own differing conventions if needed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at
formatdate
again reminded me that this package already has aparseTimestamp
function which wrapstime.Parse(time.RFC3339, ...)
to produce better error messages. It'd be nice to use that here; I think it should be a drop-in replacement.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, and I agree. I think I want Packer to do the same thing to get a similar UX.
I just updated formatdate !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto about the RFC3339 format