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

✨ Use dedicated logger #483

Merged
merged 1 commit into from
May 21, 2024
Merged

✨ Use dedicated logger #483

merged 1 commit into from
May 21, 2024

Conversation

principis
Copy link
Contributor

This PR replaces all occurrences of the root logger with a dedicated logger.

Calls to logging.<method> use the root logger. As explained in this section of the python docs, it is recommended to use a dedicated logger.

@MiWeiss
Copy link
Collaborator

MiWeiss commented May 18, 2024

Amazing, this has been on my todo list for too long 🥳

Now we'll only have to improve the overall quality of logs 😜

There seems to be some issue with the ci unrelated to your changes. I'll have a look at that in a bit

@MiWeiss
Copy link
Collaborator

MiWeiss commented May 18, 2024

Merging main now should do the trick


edit: master --> main

@MiWeiss
Copy link
Collaborator

MiWeiss commented May 18, 2024

@principis there seem to be some impacted tests. Do you mind checking und fixing them?

@principis
Copy link
Contributor Author

@principis there seem to be some impacted tests. Do you mind checking und fixing them?

Will do! Somewhere tonight 🙂.

@principis
Copy link
Contributor Author

Sorry for the delay! Fixed the test and rebased the branch.

@MiWeiss MiWeiss changed the title Use dedicated logger ✨ Use dedicated logger May 20, 2024
@principis
Copy link
Contributor Author

Fixed the black rule (forgot to commit that change).

@MiWeiss
Copy link
Collaborator

MiWeiss commented May 21, 2024

Thanks a lot for your contribution, @principis

@MiWeiss MiWeiss merged commit 214ef38 into sciunto-org:main May 21, 2024
18 checks passed
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