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

Lazy logging #462

Merged
merged 9 commits into from
Mar 6, 2025
Merged

Lazy logging #462

merged 9 commits into from
Mar 6, 2025

Conversation

GeneCodeSavvy
Copy link
Contributor

@GeneCodeSavvy GeneCodeSavvy commented Mar 3, 2025

#433 f strings replaced with recommended Lazy % style logging

@tcmitchell
Copy link
Collaborator

This looks great. I think this PR should also include deleting logging-fstring-interpolation and logging-not-lazy from setup.cfg. Do you agree, @GeneCodeSavvy ?

@GeneCodeSavvy
Copy link
Contributor Author

Done

@@ -124,7 +123,7 @@ def _build_extension_object(self, identity: str, sbol_type: str,
build_type = type_uri
break
except KeyError:
logging.warning(f'No builder found for {type_uri}')
logging.warning('No builder for %s', type_uri)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should maintain the text of the warning message unless there is a good reason to change it.

Suggested change
logging.warning('No builder for %s', type_uri)
logging.warning('No builder found for %s', type_uri)

@tcmitchell tcmitchell merged commit 0d2c40a into SynBioDex:main Mar 6, 2025
12 checks passed
@GeneCodeSavvy GeneCodeSavvy deleted the lazy_logging branch March 7, 2025 07:54
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