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

Other small fixes #32

Merged
merged 3 commits into from
Aug 18, 2023
Merged

Other small fixes #32

merged 3 commits into from
Aug 18, 2023

Conversation

XapaJIaMnu
Copy link
Contributor

I added a few fixes to make the linter happy.

@XapaJIaMnu XapaJIaMnu requested a review from jelmervdl August 17, 2023 07:24
@@ -62,4 +62,4 @@ def setup_logger(outputfilename: Optional[str] = None, loglevel: str = "INFO", d
# even if it has already been shutdown. This is why we use force=True to force recreation of logger so we can
# properly run our tests. Not the best solution, not sure if it's not prone to race conditions, but it is
# at the very least safe to use for the actual software running
logging.basicConfig(handlers=handlers, level=get_log_level(loglevel), format=loggingformat, datefmt='%Y-%m-%d %H:%M:%S', force=True, **kwargs)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand the purpose of **kwargs here?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's because you added encoding='utf-8' but the encoding argument is only available since Python 3.9. So a couple of lines above this one I only set kwargs to included the encoding bit if Python is 3.9 or higher.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, right.. Will fix. it

@jelmervdl jelmervdl merged commit 1f287a5 into main Aug 18, 2023
@jelmervdl jelmervdl deleted the other_small_fixes branch August 18, 2023 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.

2 participants