Skip to content

chore(pruner): add information to log and errors#3712

Merged
rodrodros merged 3 commits into
mainfrom
rdr/minor-pruner-improvs
Jun 8, 2026
Merged

chore(pruner): add information to log and errors#3712
rodrodros merged 3 commits into
mainfrom
rdr/minor-pruner-improvs

Conversation

@rodrodros

Copy link
Copy Markdown
Contributor

And update prune-mode value to 128

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @rodrodros's task in 1m 13s —— View job


PR Review: chore(pruner): add information to log and errors

Reviewed the diff against origin/main. The logging/error-message improvements are good. I posted 2 inline comments — both important:

important

  • pruner/pruner.go:278 — The new error message drops the underlying error (no %w), so the actual cause is lost vs. the previous return 0, err. Also "block head" → "block header" typo. inline
  • cmd/juno/juno.go:498 — Bumping NoOptDefVal to 128 leaves the pruneModeUsage help text (line 260) stale: it still says bare --prune-mode is "same as --prune-mode=0". inline

nits

  • The other error wraps (getting chain height, finding oldest block between %d and %d) and the log field renames (prunedBelow, blocksPruned, duration) look good and correctly preserve %w where a cause exists.

No correctness/concurrency/resource issues found beyond the above. The error-wrap fix at line 278 is the one I'd treat as a must-fix since it loses diagnostic info.

Comment thread pruner/pruner.go Outdated
Comment thread cmd/juno/juno.go

@EgeCaner EgeCaner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with github actions comments, rest looks good to me!

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @rodrodros's task in 1m 1s —— View job


Re-review after synchronize

Re-reviewed the latest commits (da030ae6, 19dcef64) against origin/main. Both prior important comments are now resolved:

  • pruner/pruner.go:278 — error now wraps the cause with %w and the "block head" → "block header" typo is fixed:
    return 0, fmt.Errorf("getting block header for block %d: %w", mid, err)
  • cmd/juno/juno.go:260pruneModeUsage help text updated to match the new NoOptDefVal = "128":
    --prune-mode      same as --prune-mode=128; keep 128 blocks below the pivot
    

I also confirmed the surrounding logic stays consistent: pruning is still gated on flag presence (config.Prune = v.IsSet(pruneModeF) at juno.go:390), so bumping NoOptDefVal only changes the retention window for a bare --prune-mode, not whether pruning is enabled.

Verdict: ✅ Clean — no remaining blocker/important/nit issues. Good to merge.

@codecov

codecov Bot commented Jun 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.14286% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.54%. Comparing base (3d6bcc7) to head (19dcef6).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pruner/pruner.go 53.84% 6 Missing ⚠️

❌ Your patch check has failed because the patch coverage (57.14%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3712      +/-   ##
==========================================
+ Coverage   76.21%   76.54%   +0.33%     
==========================================
  Files         405      405              
  Lines       37075    36946     -129     
==========================================
+ Hits        28255    28282      +27     
+ Misses       6827     6674     -153     
+ Partials     1993     1990       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rodrodros rodrodros merged commit e39d958 into main Jun 8, 2026
27 of 28 checks passed
@rodrodros rodrodros deleted the rdr/minor-pruner-improvs branch June 8, 2026 15:31
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.

2 participants