This repository has been archived by the owner on Feb 15, 2024. It is now read-only.
Retry delay: Update flag, vars and docs to clarify #87
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BACKSTORY As I went to start work towards implementing email notification support per GH-3, I was struck by how unclear the arguments to the `emailNotifier` function (and similarly, the `teamsNotifier` function) were. The difference between `config.NotifyMgrEmailNotificationDelay` and `cfg.EmailNotificationDelay()` were not immediately clear and it took longer than I'd like to admit to fully backtrack and confirm not only the original intent, but the current behavior surrounding those settings. This commit attempts to clarify not only the original intent, but the purpose behind each related portion of code surrounding what I now understand to be handling the rate limit and notification retry delay. CHANGES - documentation - README.md - update "feature" entry to emphasize configurable retries, but hard-coded notifications rate limit (hopefully soon to change) - configure.md - updated description - updated command-line flag name - updated environment variable name - updated help text/description for flag - doc.go - explicitly note that it is the retry delay that is configurable - configuration file - rename `delay` to `retry_delay` - add missing doc comments for each msteams section setting - code - search/replace across codebase to replace `Delay` with `RetryDelay` - this includes renaming the `Delay` field for the `MSTeams` type, and its associated flag, environment variable and TOML config file setting - replace `sendDelay` parameter with `sendRateLimit` to better communicate what the incoming value to `teamsNotifier` and `emailNotifier` is intended for - doc comments - fixes to correct wrong information - fixes to better communicate the intent and real behavior REFERENCES - refs GH-85
atc0005
added
documentation
atc0005
added a commit
that referenced
this pull request
Jul 11, 2020
SUMMARY As a follow-up to GH-87, this commit exposes a new setting that allows directly configuring the rate limit used for Teams notifications. GH-3 will be responsible for exposing the same setting for email notifications. CHANGES - update documentation to note new setting - README - GoDoc - configure - configuration file - new `rate_limit` setting - code - remove rate limit constants in favor of external settings (email rate limit setting scheduled for GH-3) and default const value if not specified externally - new `TeamsNotificationRateLimit` getter func - new `EmailNotificationRateLimit` getter func - stub only; further work scheduled for GH-3 REFERENCES refs GH-87, GH-85, GH-19, GH-3
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BACKSTORY
As I went to start work towards implementing email notification
support per GH-3, I was struck by how unclear the arguments to the
emailNotifierfunction (and similarly, theteamsNotifierfunction)were. The difference between
config.NotifyMgrEmailNotificationDelayand
cfg.EmailNotificationDelay()were not immediately clear and ittook longer than I'd like to admit to fully backtrack and confirm
not only the original intent, but the current behavior surrounding
those settings.
This commit attempts to clarify not only the original intent, but the
purpose behind each related portion of code surrounding what I now
understand to be handling the rate limit and notification retry
delay.
CHANGES
documentation
hard-coded notifications rate limit (hopefully soon to change)
configuration file
delaytoretry_delaycode
search/replace across codebase to replace
DelaywithRetryDelayDelayfield for theMSTeamstype,and its associated flag, environment variable and TOML config
file setting
replace
sendDelayparameter withsendRateLimitto bettercommunicate what the incoming value to
teamsNotifierandemailNotifieris intended fordoc comments
REFERENCES