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

refactor: trim log messages and remove walrus operators #362

Merged
merged 1 commit into from
Dec 5, 2023
Merged

Conversation

maxrake
Copy link
Contributor

@maxrake maxrake commented Dec 5, 2023

This change seeks to clean up a few things that were discovered during recent testing, and to do so before the next release. The use of walrus operators (i.e., :=) has been removed in favor of more, but simpler, code. Multiline log messages have been trimmed a bit after it was discovered that lines with >71 characters wrap in constrained environments where the WIDTH is fixed at 80 characters (e.g., GitHub Actions). This is due to the space between the log level (e.g., WARNING) and the message being two characters instead of the assumed one, allowing a max of 71 characters for each line of the message instead of 72 as previously assumed. There is likely a better way to programmatically account for this with something in the textwrap module, but these small changes will do for now.

This change seeks to clean up a few things that were discovered during
recent testing, and to do so before the next release. The use of walrus
operators (i.e., `:=`) has been removed in favor of more, but simpler,
code. Multiline log messages have been trimmed a bit after it was
discovered that lines with >71 characters wrap in constrained
environments where the WIDTH is fixed at 80 characters (e.g., GitHub
Actions). This is due to the space between the log level (e.g.,
`WARNING`) and the message being two characters instead of the assumed
one, allowing a max of 71 characters for each line of the message
instead of 72 as previously assumed. There is likely a better way to
programmatically account for this with something in the `textwrap`
module, but these small changes will do for now.
@maxrake maxrake requested a review from kylewillmon December 5, 2023 18:21
@maxrake maxrake self-assigned this Dec 5, 2023
@maxrake maxrake requested a review from a team as a code owner December 5, 2023 18:21
@maxrake maxrake merged commit 50772b4 into main Dec 5, 2023
10 checks passed
@maxrake maxrake deleted the cleanups branch December 5, 2023 18:59
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