From a2d0bd981dc260515316710a686dbbba8b78da88 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 22 Mar 2026 23:12:59 +0000 Subject: [PATCH 1/3] refactor(parsed_value): remove unnecessary `Error` derive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ParsedValue` is not an error type — it's a display wrapper for formatted byte values. Remove the unused `Error` derive and update documentation to clarify that `Display` and `Error` should only be derived when actually used. https://claude.ai/code/session_012YdPMpQ3UiSmqFSU2vUULm --- .github/copilot-instructions.md | 2 +- AGENTS.md | 2 +- CLAUDE.md | 2 +- CONTRIBUTING.md | 5 ++++- src/bytes_format/parsed_value.rs | 4 ++-- template/ai-instructions/shared.md | 2 +- 6 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 7afa4d56..955ecd49 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -12,7 +12,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c - Use `pipe-trait` for chaining through unary functions (constructors, `Some`, `Ok`, free functions, etc.), avoiding nested calls, and continuing method chains — but not for simple standalone calls (prefer `foo(value)` over `value.pipe(foo)`) - Prefer `where` clauses for multiple trait bounds - Derive order: std traits → comparison traits → `Hash` → derive_more → feature-gated -- Custom errors: `#[derive(Debug, Display, Error)]` +- Error types: only derive `Display` and `Error` from `derive_more` when each trait is actually used — not all displayable types need `Error`, and not all error types need `Display` - Minimize `unwrap()` in non-test code — use proper error handling - Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy` - Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes diff --git a/AGENTS.md b/AGENTS.md index 7afa4d56..955ecd49 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,7 +12,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c - Use `pipe-trait` for chaining through unary functions (constructors, `Some`, `Ok`, free functions, etc.), avoiding nested calls, and continuing method chains — but not for simple standalone calls (prefer `foo(value)` over `value.pipe(foo)`) - Prefer `where` clauses for multiple trait bounds - Derive order: std traits → comparison traits → `Hash` → derive_more → feature-gated -- Custom errors: `#[derive(Debug, Display, Error)]` +- Error types: only derive `Display` and `Error` from `derive_more` when each trait is actually used — not all displayable types need `Error`, and not all error types need `Display` - Minimize `unwrap()` in non-test code — use proper error handling - Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy` - Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes diff --git a/CLAUDE.md b/CLAUDE.md index f27d3302..997932e0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,7 +12,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c - Use `pipe-trait` for chaining through unary functions (constructors, `Some`, `Ok`, free functions, etc.), avoiding nested calls, and continuing method chains — but not for simple standalone calls (prefer `foo(value)` over `value.pipe(foo)`) - Prefer `where` clauses for multiple trait bounds - Derive order: std traits → comparison traits → `Hash` → derive_more → feature-gated -- Custom errors: `#[derive(Debug, Display, Error)]` +- Error types: only derive `Display` and `Error` from `derive_more` when each trait is actually used — not all displayable types need `Error`, and not all error types need `Display` - Minimize `unwrap()` in non-test code — use proper error handling - Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy` - Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 10d1b7e4..f862544c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -198,7 +198,10 @@ where ### Error Handling -- Define custom error enums with `#[derive(Debug, Display, Error)]` from `derive_more`. +- Use `derive_more` for error types. Only derive the traits that are actually used: + - `Display`: derive when the type needs to be displayed (e.g., printed to stderr, used in format strings). + - `Error`: derive when the type is used as a `std::error::Error` (e.g., as the error type in `Result`, or as a source of another error). Not all types with `Display` need `Error`. + - A type that only needs formatting (not error handling) should derive `Display` without `Error`. - Minimize `unwrap()` in non-test code — use proper error propagation. `unwrap()` is acceptable in tests and for provably infallible operations (with a comment explaining why). When deliberately ignoring an error, use `.ok()` with a comment explaining why. ```rust diff --git a/src/bytes_format/parsed_value.rs b/src/bytes_format/parsed_value.rs index 89d8d3e0..d8aad461 100644 --- a/src/bytes_format/parsed_value.rs +++ b/src/bytes_format/parsed_value.rs @@ -1,7 +1,7 @@ -use derive_more::{Display, Error}; +use derive_more::Display; /// Return value of [`Formatter::parse_value`](super::Formatter::parse_value). -#[derive(Debug, Display, Clone, Copy, Error)] +#[derive(Debug, Display, Clone, Copy)] pub enum ParsedValue { /// When input value is less than `scale_base`. #[display("{value} ")] diff --git a/template/ai-instructions/shared.md b/template/ai-instructions/shared.md index 7afa4d56..955ecd49 100644 --- a/template/ai-instructions/shared.md +++ b/template/ai-instructions/shared.md @@ -12,7 +12,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c - Use `pipe-trait` for chaining through unary functions (constructors, `Some`, `Ok`, free functions, etc.), avoiding nested calls, and continuing method chains — but not for simple standalone calls (prefer `foo(value)` over `value.pipe(foo)`) - Prefer `where` clauses for multiple trait bounds - Derive order: std traits → comparison traits → `Hash` → derive_more → feature-gated -- Custom errors: `#[derive(Debug, Display, Error)]` +- Error types: only derive `Display` and `Error` from `derive_more` when each trait is actually used — not all displayable types need `Error`, and not all error types need `Display` - Minimize `unwrap()` in non-test code — use proper error handling - Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy` - Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes From be21889de5498459a1e2382b501770c86cf6d04e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 22 Mar 2026 23:13:36 +0000 Subject: [PATCH 2/3] docs: clarify when to derive `Display` and `Error` from `derive_more` Update CONTRIBUTING.md and AI instruction templates to clarify that `Display` and `Error` should only be derived when each trait is actually used, rather than always bundling them together as `#[derive(Debug, Display, Error)]`. https://claude.ai/code/session_012YdPMpQ3UiSmqFSU2vUULm --- src/bytes_format/parsed_value.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bytes_format/parsed_value.rs b/src/bytes_format/parsed_value.rs index d8aad461..89d8d3e0 100644 --- a/src/bytes_format/parsed_value.rs +++ b/src/bytes_format/parsed_value.rs @@ -1,7 +1,7 @@ -use derive_more::Display; +use derive_more::{Display, Error}; /// Return value of [`Formatter::parse_value`](super::Formatter::parse_value). -#[derive(Debug, Display, Clone, Copy)] +#[derive(Debug, Display, Clone, Copy, Error)] pub enum ParsedValue { /// When input value is less than `scale_base`. #[display("{value} ")] From eab6f6cac151ecaad33b0de8a738dd7ec8108c1f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 23 Mar 2026 00:17:12 +0000 Subject: [PATCH 3/3] docs: fix misleading claim about Error not needing Display `std::error::Error` requires `Display` as a supertrait, so saying "not all error types need Display" was inaccurate. Simplify the guideline to focus on the valid point: not all displayable types are errors. https://claude.ai/code/session_012YdPMpQ3UiSmqFSU2vUULm --- .github/copilot-instructions.md | 2 +- AGENTS.md | 2 +- CLAUDE.md | 2 +- template/ai-instructions/shared.md | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 955ecd49..7ef6dd66 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -12,7 +12,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c - Use `pipe-trait` for chaining through unary functions (constructors, `Some`, `Ok`, free functions, etc.), avoiding nested calls, and continuing method chains — but not for simple standalone calls (prefer `foo(value)` over `value.pipe(foo)`) - Prefer `where` clauses for multiple trait bounds - Derive order: std traits → comparison traits → `Hash` → derive_more → feature-gated -- Error types: only derive `Display` and `Error` from `derive_more` when each trait is actually used — not all displayable types need `Error`, and not all error types need `Display` +- Error types: only derive `Display` and `Error` from `derive_more` when each is actually needed — not all displayable types are errors - Minimize `unwrap()` in non-test code — use proper error handling - Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy` - Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes diff --git a/AGENTS.md b/AGENTS.md index 955ecd49..7ef6dd66 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,7 +12,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c - Use `pipe-trait` for chaining through unary functions (constructors, `Some`, `Ok`, free functions, etc.), avoiding nested calls, and continuing method chains — but not for simple standalone calls (prefer `foo(value)` over `value.pipe(foo)`) - Prefer `where` clauses for multiple trait bounds - Derive order: std traits → comparison traits → `Hash` → derive_more → feature-gated -- Error types: only derive `Display` and `Error` from `derive_more` when each trait is actually used — not all displayable types need `Error`, and not all error types need `Display` +- Error types: only derive `Display` and `Error` from `derive_more` when each is actually needed — not all displayable types are errors - Minimize `unwrap()` in non-test code — use proper error handling - Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy` - Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes diff --git a/CLAUDE.md b/CLAUDE.md index 997932e0..e9be8075 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,7 +12,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c - Use `pipe-trait` for chaining through unary functions (constructors, `Some`, `Ok`, free functions, etc.), avoiding nested calls, and continuing method chains — but not for simple standalone calls (prefer `foo(value)` over `value.pipe(foo)`) - Prefer `where` clauses for multiple trait bounds - Derive order: std traits → comparison traits → `Hash` → derive_more → feature-gated -- Error types: only derive `Display` and `Error` from `derive_more` when each trait is actually used — not all displayable types need `Error`, and not all error types need `Display` +- Error types: only derive `Display` and `Error` from `derive_more` when each is actually needed — not all displayable types are errors - Minimize `unwrap()` in non-test code — use proper error handling - Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy` - Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes diff --git a/template/ai-instructions/shared.md b/template/ai-instructions/shared.md index 955ecd49..7ef6dd66 100644 --- a/template/ai-instructions/shared.md +++ b/template/ai-instructions/shared.md @@ -12,7 +12,7 @@ Read and follow the CONTRIBUTING.md file in this repository for all code style c - Use `pipe-trait` for chaining through unary functions (constructors, `Some`, `Ok`, free functions, etc.), avoiding nested calls, and continuing method chains — but not for simple standalone calls (prefer `foo(value)` over `value.pipe(foo)`) - Prefer `where` clauses for multiple trait bounds - Derive order: std traits → comparison traits → `Hash` → derive_more → feature-gated -- Error types: only derive `Display` and `Error` from `derive_more` when each trait is actually used — not all displayable types need `Error`, and not all error types need `Display` +- Error types: only derive `Display` and `Error` from `derive_more` when each is actually needed — not all displayable types are errors - Minimize `unwrap()` in non-test code — use proper error handling - Install toolchain before running tests: `rustup toolchain install "$(< rust-toolchain)" && rustup component add --toolchain "$(< rust-toolchain)" rustfmt clippy` - Run `FMT=true LINT=true BUILD=true TEST=true DOC=true ./test.sh` to validate changes