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

chore(pruner): remove SpacedHeaderGenerator #4032

Merged
merged 3 commits into from
Jan 15, 2025

Conversation

ronething-bot
Copy link
Contributor

closes #3278

This is the first time I'm contributing to the project, and I'm not very familiar with it yet. If i missing something, please left a review comment, thanks.

@github-actions github-actions bot added the external Issues created by non node team members label Jan 6, 2025
@renaynay renaynay self-assigned this Jan 7, 2025
@codecov-commenter
Copy link

codecov-commenter commented Jan 10, 2025

Codecov Report

Attention: Patch coverage is 38.46154% with 8 lines in your changes missing coverage. Please review.

Project coverage is 45.55%. Comparing base (2469e7a) to head (b372d4b).
Report is 415 commits behind head on main.

Files with missing lines Patch % Lines
header/headertest/testing.go 38.46% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4032      +/-   ##
==========================================
+ Coverage   44.83%   45.55%   +0.71%     
==========================================
  Files         265      309      +44     
  Lines       14620    22184    +7564     
==========================================
+ Hits         6555    10105    +3550     
- Misses       7313    11007    +3694     
- Partials      752     1072     +320     

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

Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution @ronething-bot !

The only request i have is to rename the constructor for the test suite to NewTestSuiteWithGenesisTime

@ronething-bot
Copy link
Contributor Author

@renaynay done, please take a review again, thanks.

Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

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

Nice! Thanks @ronething-bot

@ronething-bot
Copy link
Contributor Author

@renaynay Required Labels / label (pull_request), CI failed, can you take a look, thanks?

Copy link
Contributor

@cristaloleg cristaloleg left a comment

Choose a reason for hiding this comment

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

SGTM

@ronething-bot
Copy link
Contributor Author

@renaynay @cristaloleg hi, can we merge this PR now?

@renaynay renaynay enabled auto-merge (squash) January 15, 2025 08:36
@renaynay renaynay merged commit c599285 into celestiaorg:main Jan 15, 2025
22 of 23 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
external Issues created by non node team members kind:chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chore(pruner | headertest): Deduplicate header generator utilities
4 participants