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

Reduce logging verbosity #228

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ClementNerma
Copy link
Contributor

This will prevent polluting other applications' logs with informations that are better suited to the debug! or trace! macro.

I went for trace! as I think it suits this role better, besides debug! is already used in the crate for some other information types.

@pdeljanov
Copy link
Owner

Sorry, I don't think I will merge this as-is.

Perhaps we can reduce the verbosity of a few choice logs, but since there are also changes to our internal testing tools: symphonia-play and symphonia-check this seems like a mass find-and-replace.

It is going to be a losing battle to try and control the logs of all your dependencies. You should instead use the filtering options provided by env_logger (or similar) to either only show your app's own logs, or to limit the verbosity of your dependencies. Generally, warn! and error! are used rather sparingly in Symphonia for that reason.

@ClementNerma
Copy link
Contributor Author

I think it would really be better to remove the usages of info! as I feel it's used wrongly in the codebase, but if the tests are also depending on it I'll look into that.

@pdeljanov
Copy link
Owner

I think it would really be better to remove the usages of info! as I feel it's used wrongly in the codebase, but if the tests are also depending on it I'll look into that.

Could you please provide some reasoning for why it is "wrong"?

I can certainly believe some usages may be better suited as trace!, however, others involve logging when audible glitches may occur (e.g., count1 errors).

Could you reformat this PR with the nightly toolchain and then I can go over what has been changed. Maybe we can lower the verbosity for some messages.

@ClementNerma
Copy link
Contributor Author

Could you please provide some reasoning for why it is "wrong"?

Messages that only provide informations useful to debug the program should be set as debug! or trace!.

Many of the informations shown here use info! even though they don't have the same importance level as an actual user information.

I may be wrong but I feel like it would be better that way.

I'll reformat this PR so you can check it :)

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.

None yet

2 participants