Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Implement Debug trait for public structs #73

Closed
zeodtr opened this issue Oct 4, 2023 · 1 comment · Fixed by #92
Closed

Implement Debug trait for public structs #73

zeodtr opened this issue Oct 4, 2023 · 1 comment · Fixed by #92
Labels
good first issue Good for newcomers

Comments

@zeodtr
Copy link

zeodtr commented Oct 4, 2023

It's useful to implement Debug trait for public structs.

The Rust document strongly recommends it: https://doc.rust-lang.org/std/fmt/index.html#fmtdisplay-vs-fmtdebug
Also, I found that there is a lint directive for it: https://doc.rust-lang.org/stable/nightly-rustc/rustc_lint/builtin/static.MISSING_DEBUG_IMPLEMENTATIONS.html
Maybe it would be nice to include that lint directive as deny to lib.rs.

Also, can traits like Catalog be modified to be dependent upon Debug trait like this?

pub trait Catalog: Debug {

It would be convenient when including a Box dyned trait member to a struct that wants to implement Debug trait. Otherwise, since Debug trait is not an auto trait, we must create a new wrapper trait only to specify Debug trait dependency.

Also, (maybe this is somewhat irrelevant to this issue. If so, ignore this) it would be convenient if Catalog trait be modified to be dependent also upon Send and Sync like this:

pub trait Catalog: Debug + Send + Sync {

Since Catalog trait is an async trait, adding Send and Sync can be useful. (BTW, I've added them to a trait that wraps the Catalog trait to include Debug trait.)

Thanks.

@zeodtr zeodtr changed the title Implement Debug trait for public structs Implement Debug trait for public structs Oct 4, 2023
@liurenjie1024
Copy link
Collaborator

+1 for this proposal.

@liurenjie1024 liurenjie1024 added the good first issue Good for newcomers label Nov 1, 2023
DeaconDesperado added a commit to DeaconDesperado/iceberg-rust that referenced this issue Nov 4, 2023
DeaconDesperado added a commit to DeaconDesperado/iceberg-rust that referenced this issue Nov 4, 2023
DeaconDesperado added a commit to DeaconDesperado/iceberg-rust that referenced this issue Nov 4, 2023
DeaconDesperado added a commit to DeaconDesperado/iceberg-rust that referenced this issue Nov 4, 2023
DeaconDesperado added a commit to DeaconDesperado/iceberg-rust that referenced this issue Nov 4, 2023
DeaconDesperado added a commit to DeaconDesperado/iceberg-rust that referenced this issue Nov 4, 2023
DeaconDesperado added a commit to DeaconDesperado/iceberg-rust that referenced this issue Nov 8, 2023
@Fokko Fokko closed this as completed in #92 Nov 13, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants