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

Replace string interpolation with log arguments and f-strings #254

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

webbnh
Copy link
Collaborator

@webbnh webbnh commented Dec 5, 2024

This PR is the next in the series of refactorings originally set out in #207 which endeavors to replace duplicated code in Sync2Jira with common subroutines; this is the follow-on to #253.

These changes center on removing the uses of Python2-style string interpolation, replacing them with more appropriate operations.

For inserting values into log messages, the logger will do the interpolation automatically if the values are provided as arguments to the call. This approach has the added advantage that the interpolation is deferred until the message is actually logged, such that, if the intended class of logging is disabled (e.g., calling log.debug() when the log level is set to "INFO") the interpolation will be skipped, since the message won't be logged, which is a performance savings. So, this PR removes uses of the interpolation operator (%) from logging calls instead passing the to-be-interpolated values as arguments to the logger call.

For inserting values into other strings, Python3 offers "f-strings" which are generally more expressive and easier to read than the Python2 format arguments used with the interpolation operator. So, this PR makes that substitution where it improves the readability of the code.

For converting values to strings without further augmentation, this PR replaces the interpolation with a call to str().

Added code to trim whitespace from the issue owner, iff the owner is not None, blank, or in some other way absent or unpleasant. (A similar change for the admin username is coming in a subsequent commit.)

And, removed a couple of unneeded list instantiations.

@ralphbean ralphbean merged commit e353406 into main Dec 6, 2024
6 checks passed
@ralphbean ralphbean deleted the replace-string-interpolation branch December 6, 2024 02:36
@ralphbean
Copy link
Member

Makes total sense. :) Thanks for cranking on this @webbnh.

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