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

Include unit when logging base delay option #113

Closed
wants to merge 1 commit into from

Conversation

ruuda
Copy link
Contributor

@ruuda ruuda commented Oct 6, 2021

Logging just "Base delay: 40" is not very helpful. 40 what? Seconds? Milliseconds? Martian days? Clarify that by including the unit.

Before:

$ vaultenv --log-level info --secrets-file secrets env
...
Base delay:            40
...

After:

$ vaultenv --log-level info --secrets-file secrets env
...
Base delay:            40 ms
...

Just "Base delay: 40" is not very helpful. 40 what? Seconds?
Milliseconds? Martian days? Clarify that by including the unit.
Copy link

@rkrzr rkrzr left a comment

Choose a reason for hiding this comment

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

LGTM

@maartenberg
Copy link
Member

@OpsBotPrime merge

@OpsBotPrime
Copy link
Contributor

Pull request approved for merge by @maartenberg, waiting for rebase behind 2 pull requests.

@OpsBotPrime
Copy link
Contributor

Rebased as 5c0407a, waiting for CI …

@maartenberg
Copy link
Member

Closing: This PR has been merged to master in 5c0407a, but because it came from an external repository our mergebot seems to have trouble getting Github to recognize that this PR has been merged.

Thanks for adding this!

@ruuda ruuda deleted the base-delay-unit branch October 15, 2021 10:26
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.

4 participants