Skip to content
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

Add support for multiple offsets to time_offset module #189

Merged

Conversation

sumkincpp
Copy link
Contributor

Add support for multiple offsets to time_offset

resource "time_offset" "example" {
  offset_years = 1
  offset_months = 1
}
output "one_year_and_month_from_now" {
  value = time_offset.example.rfc3339
}

Workaround right now, for example - use only one attribute, i.e. offset_months=13

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

@sumkincpp sumkincpp requested a review from a team as a code owner April 3, 2023 11:18
@hashicorp-cla
Copy link

hashicorp-cla commented Apr 3, 2023

CLA assistant check
All committers have signed the CLA.

@SBGoods
Copy link
Contributor

SBGoods commented Jun 13, 2023

Hi @sumkincpp thank you for the contribution,
Since we currently allow practitioners to set multiple offsets in the configuration, there is a possibility that some practitioners may have these set inadvertently and this feature would result in a breaking change for them. We can consider this PR as part of a larger effort for a Time Provider major release in the future but we do not have a timeline for this at the moment. Feel free to keep this PR open if you would like, but we might not be able to review this PR in the near future.

@pascal-hofmann
Copy link

pascal-hofmann commented Oct 6, 2023

This provider has four resources and a major bug in one of them which cost me quite some time today. Why not just merge and release a new major version?

I understand that for things like terraform-provider-aws pushing new major versions requires some more planning, but I don't think it makes sense here.

Copy link
Contributor

@SBGoods SBGoods left a comment

Choose a reason for hiding this comment

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

Hi @sumkincpp
I've added a changelog entry and moved your doc example to the template to pass our CI check. Otherwise, this looks good to go.

Thank you for the contribution!

@SBGoods SBGoods linked an issue Nov 30, 2023 that may be closed by this pull request
1 task
@SBGoods SBGoods merged commit 00d68c5 into hashicorp:main Nov 30, 2023
6 checks passed
@pascal-hofmann
Copy link

Thanks for merging this! 🚀

@bflad bflad added this to the v0.10.0 milestone Dec 1, 2023
@bflad bflad added the bug Something isn't working label Dec 1, 2023
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

time_offset doesn't merge when multiple offsets are specified
5 participants