chore: add comment section for claude (#19108)

This commit is contained in:
Matthias Seitz
2025-10-17 16:45:24 +02:00
committed by GitHub
parent 1634535e00
commit 928d91dbf9

View File

@@ -236,6 +236,85 @@ Common refactoring pattern:
- Add trait bounds for flexibility
- Enable reuse across different chain types (Ethereum, Optimism)
#### When to Comment
Write comments that remain valuable after the PR is merged. Future readers won't have PR context - they only see the current code.
##### ✅ DO: Add Value
**Explain WHY and non-obvious behavior:**
```rust
// Process must handle allocations atomically to prevent race conditions
// between dealloc on drop and concurrent limit checks
unsafe impl GlobalAlloc for LimitedAllocator { ... }
// Binary search requires sorted input. Panics on unsorted slices.
fn find_index(items: &[Item], target: &Item) -> Option
// Timeout set to 5s to match EVM block processing limits
const TRACER_TIMEOUT: Duration = Duration::from_secs(5);
```
**Document constraints and assumptions:**
```rust
/// Returns heap size estimate.
///
/// Note: May undercount shared references (Rc/Arc). For precise
/// accounting, combine with an allocator-based approach.
fn deep_size_of(&self) -> usize
```
**Explain complex logic:**
```rust
// We reset limits at task start because tokio reuses threads in
// spawn_blocking pool. Without reset, second task inherits first
// task's allocation count and immediately hits limit.
THREAD_ALLOCATED.with(|allocated| allocated.set(0));
```
##### ❌ DON'T: Describe Changes
```rust
// ❌ BAD - Describes the change, not the code
// Changed from Vec to HashMap for O(1) lookups
// ✅ GOOD - Explains the decision
// HashMap provides O(1) symbol lookups during trace replay
```
```rust
// ❌ BAD - PR-specific context
// Fix for issue #234 where memory wasn't freed
// ✅ GOOD - Documents the actual behavior
// Explicitly drop allocations before limit check to ensure
// accurate accounting
```
```rust
// ❌ BAD - States the obvious
// Increment counter
counter += 1;
// ✅ GOOD - Explains non-obvious purpose
// Track allocations across all threads for global limit enforcement
GLOBAL_COUNTER.fetch_add(1, Ordering::SeqCst);
```
✅ **Comment when:**
- Non-obvious behavior or edge cases
- Performance trade-offs
- Safety requirements (unsafe blocks must always be documented)
- Limitations or gotchas
- Why simpler alternatives don't work
❌ **Don't comment when:**
- Code is self-explanatory
- Just restating the code in English
- Describing what changed in this PR
##### The Test: "Will this make sense in 6 months?"
Before adding a comment, ask: Would someone reading just the current code (no PR, no history) find this helpful?
### Example Contribution Workflow
Let's say you want to fix a bug where external IP resolution fails on startup: