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

Calls to println/eprintln #32

Open
ettom opened this issue Jan 3, 2024 · 2 comments
Open

Calls to println/eprintln #32

ettom opened this issue Jan 3, 2024 · 2 comments

Comments

@ettom
Copy link
Contributor

ettom commented Jan 3, 2024

It would be nice if the library didn't just print stuff to stdout/stderr indiscriminately. AFAIK the idiomatic solution would be to use the log or tracing crate.

@njaard
Copy link
Owner

njaard commented Jan 4, 2024

I agree! There are only a few cases of eprintln in the code. Many of them can be removed harmlessly.

The "disregarding database" error should be part of the user interface.

The biggest question is what to do about warning: invalid segment version. Is that a corrupt or malformed file a valid use for a panic?

@ettom
Copy link
Contributor Author

ettom commented Jan 4, 2024

I think that the library itself should never call (e)println and ideally it wouldn't panic either. In both cases, doing otherwise is taking control away from the application developer. What about propagating the disregarding database or invalid segment version and similar errors as Errs, with a custom Error type? For this, the thiserror create could help. You could have different variants with some of them being marked as recoverable and others as fatal, where appropriate.

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

No branches or pull requests

2 participants