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

(IAC-918) - disable_time_zone_synchronization function implemented #145

Merged
merged 1 commit into from
Jul 22, 2020

Conversation

david22swan
Copy link
Member

No description provided.

@david22swan david22swan requested a review from a team as a code owner July 21, 2020 14:36
@puppet-community-rangefinder
Copy link

scheduled_task is a type

Breaking changes to this file MAY impact these 10 modules (near match):

This module is declared in 0 of 575 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2020

Codecov Report

Merging #145 into master will decrease coverage by 0.10%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #145      +/-   ##
==========================================
- Coverage   95.42%   95.31%   -0.11%     
==========================================
  Files           6        6              
  Lines         830      833       +3     
==========================================
+ Hits          792      794       +2     
- Misses         38       39       +1     
Impacted Files Coverage Δ
lib/puppet/type/scheduled_task.rb 92.85% <ø> (ø)
lib/puppet_x/puppetlabs/scheduled_task/trigger.rb 94.75% <75.00%> (-0.28%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55d2d35...923ea77. Read the comment docs.

@@ -140,6 +140,8 @@ def insync?(current)
* `minutes_interval` --- The repeat interval in minutes.
* `minutes_duration` --- The duration in minutes, needs to be greater than the
minutes_interval.
* `disable_synchronise` --- Whether or not to disable the `synchronise across
Copy link
Contributor

Choose a reason for hiding this comment

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

I would be tempted to name it something a bit more in line with the Windows functionality it's controlling. Perhaps time_zone_synchronization, or better still, exactly as it is in the UI: synchronize_across_time_zones. I will admit that's a bit long, but it will make it obvious what it's related to.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to disable_time_zone_synchronization. synchronize_across_time_zones doesn't really work due to the function turning it of rather than varying it or turning it on.

apply_manifest(pp, catch_failures: true)

# Ensure it's idempotent
# apply_manifest(pp, catch_changes: true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be deleted?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed and added a note to the type to show that it is non-idempotent

apply_manifest(pp, catch_failures: true)

# Ensure it's idempotent
# apply_manifest(pp, catch_changes: true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be deleted?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed and added a note to the type to show that it is non-idempotent

spec/acceptance/should_create_task_spec.rb Show resolved Hide resolved
@david22swan david22swan changed the title (IAC-918) - disable_synchronise function implemented (IAC-918) - disable_time_zone_synchronization function implemented Jul 21, 2020
@sanfrancrisko sanfrancrisko changed the title (IAC-918) - disable_time_zone_synchronization function implemented (IAC-918) - disable_time_zone_synchronization function implemented Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants