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

resource/pagerduty_schedule: suppress spurious start diff #343

Merged
merged 1 commit into from
Jun 18, 2021

Conversation

Padorax
Copy link
Contributor

@Padorax Padorax commented Jun 8, 2021

Issue: #200

When both schedule layer start time (the old one read from PagerDuty API, and the new one read
from configuration files) are in the past, there is no need to show Diff as the acutual start
time will be updated by PagerDuty API to current time.

Otherwise, users will have to sync the configuration files from time to time, when they make
irrelevant changes to their schedules (e.g., adding a user) or even no changes at all.

Issue: PagerDuty#200

When both schedule layer start time (the old one read from PagerDuty API, and the new one read
from configuration files) are in the past, there is no need to show Diff as the acutual start
time will be updated by PagerDuty API to current time.

Otherwise, users will have to sync the configuration files from time to time, when they make
irrelevant changes to their schedules (e.g., adding a user) or even no changes at all.
Copy link
Contributor

@drastawi drastawi left a comment

Choose a reason for hiding this comment

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

This is great @Padorax. Hopefully it can get merged soon. cc @stmcallister

Copy link
Contributor

@stmcallister stmcallister left a comment

Choose a reason for hiding this comment

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

Thanks for this! Looks great! 🎉 👍

@stmcallister stmcallister merged commit 765cdbb into PagerDuty:master Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants