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

Change EventScheduleRate parameter to EventSchedulePeriod and require units #102

Conversation

triarius
Copy link
Contributor

@triarius triarius commented Aug 28, 2023

The rate expression must pluralise the unit, so 1 minute and 0 minutes are valid, but 0 minute is not.

Rather than condition on the value of the parameter, we leave this to
the user to specify whether they should use "minute" or "minutes". This
also allows them to use other units like 1 hour, 1 day, etc. Though
units that are longer or shorter than a minute are not recommended.

Since the old parameter was essentially broken (it would not succeed with any value other than the default, 1), we've decided to rename the parameter to EventSchedulePeriod. This name is more accurate - a length of time like 1 minute is not a rate.

… require units

The rate expression must pluralise the unit, so `1 minute` and `0 minutes` are valid, but `0 minute` is not.

Rather than condition on the value of the parameter, we leave this to
the user to specify whether they should use "minute" or "minutes". This
also allows them to use other units like 1 hour, 1 day, etc. Though
units that are longer or shorter than a minute are not recommended.
@triarius triarius force-pushed the pdp-1417-buildkite-agent-scaler-event-schedule-rate-parameter-should branch from bcb432f to e8d2d80 Compare August 31, 2023 10:25
@triarius triarius changed the title Change EventScheduleRate parameter to EventSchedulePeriod and fix rate expression Change EventScheduleRate parameter to EventSchedulePeriod and make require units Aug 31, 2023
@triarius triarius changed the title Change EventScheduleRate parameter to EventSchedulePeriod and make require units Change EventScheduleRate parameter to EventSchedulePeriod and require units Sep 1, 2023
@triarius triarius requested a review from a team September 1, 2023 06:51
Copy link
Contributor

@DrJosh9000 DrJosh9000 left a comment

Choose a reason for hiding this comment

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

🙌

@triarius triarius merged commit f0536eb into master Sep 13, 2023
1 check passed
@triarius triarius deleted the pdp-1417-buildkite-agent-scaler-event-schedule-rate-parameter-should branch September 13, 2023 01:48
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.

2 participants