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

Make disable_time_zone_synchronization idempotent #207

Conversation

omolenkamp
Copy link
Contributor

No description provided.

@omolenkamp omolenkamp requested a review from a team as a code owner March 31, 2022 08:52
@CLAassistant
Copy link

CLAassistant commented Mar 31, 2022

CLA assistant check
All committers have signed the CLA.

@chelnak
Copy link
Contributor

chelnak commented Mar 31, 2022

Hey @omolenkamp, thanks for the contribution!

Please could you provide a little more context around this change?

Also it would be great if you could add a test or two for the new functionality.

Let us know if you need any help.

@omolenkamp
Copy link
Contributor Author

disable_time_zone_synchronization works by setting StartBoundary to a local timestamp (no zone offset) when true, and using a timestamp including a zone offset when false. Because of this, the current value of disable_time_zone_synchronization can be determined by testing if StartBoundary includes a zone offset. (Z, +xx:xx, or -xx:xx)

(start_boundary cannot be used here, as the Ruby Time class doesn't differentiate between local and zoned timestamps)

@chelnak chelnak closed this May 16, 2022
@chelnak chelnak reopened this May 16, 2022
@chelnak chelnak force-pushed the disable_time_zone_synchronization-idempotent branch 4 times, most recently from 949948d to 9e7c28d Compare May 16, 2022 14:57
@chelnak chelnak force-pushed the disable_time_zone_synchronization-idempotent branch from 9e7c28d to 1f7169a Compare May 16, 2022 15:01
@chelnak
Copy link
Contributor

chelnak commented May 16, 2022

Hey @omolenkamp, thanks for updating this PR.

There were a few spec failures. I've fixed these to help move things along. If our CI passes I'll get this merged.

@chelnak chelnak merged commit 67473dc into puppetlabs:main May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants