Skip to content

Conversation

@LesnyRumcajs
Copy link
Member

@LesnyRumcajs LesnyRumcajs commented Jan 26, 2026

Summary of changes

Changes introduced in this pull request:

  • more derive_more zen

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Summary by CodeRabbit

  • Refactor
    • Enabled derived iterator behavior for a core key type and removed a custom iterator implementation for a collection.
    • Simplified internal iteration mechanics, reducing implementation complexity while preserving existing external behavior and compatibility.

✏️ Tip: You can customize this high-level summary in your review settings.

@LesnyRumcajs LesnyRumcajs requested a review from a team as a code owner January 26, 2026 13:45
@LesnyRumcajs LesnyRumcajs requested review from akaladarshi and hanabi1224 and removed request for a team January 26, 2026 13:45
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 26, 2026

Walkthrough

TipsetKey now derives IntoIterator via derive_more with an #[into_iterator(owned, ref)] on its inner field; CidHashSet drops its manual IntoIter and IntoIterator impls, relying on the derived iterator behavior.

Changes

Cohort / File(s) Summary
TipsetKey (derive_intoiterator)
src/blocks/tipset.rs
Added derive_more::IntoIterator to TipsetKey and annotated inner field: pub struct TipsetKey(#[into_iterator(owned, ref)] SmallCidNonEmptyVec);. Derive list expanded accordingly.
CidHashSet (remove manual iterator)
src/cid_collections/hash_set.rs
Removed manual IntoIter struct and its impl Iterator, and removed the impl IntoIterator for CidHashSet; iteration now uses derived/standard iterator paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • akaladarshi
  • hanabi1224
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adopting derive_more::IntoIterator to replace manual iterator implementations across the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

hanabi1224
hanabi1224 previously approved these changes Jan 26, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/cid_collections/hash_set.rs`:
- Around line 13-16: The derived derive_more::IntoIterator on CidHashSet yields
(Cid, ()) tuples from the inner CidHashMap and breaks consumers expecting Item =
Cid; remove the derive for IntoIterator and add custom IntoIterator impls for
CidHashSet and for &CidHashSet: implement IntoIterator for CidHashSet with type
Item = Cid and IntoIter = std::collections::hash_map::IntoKeys<Cid, ()>
returning self.inner.into_keys(), and implement IntoIterator for &'a CidHashSet
with type Item = &'a Cid and IntoIter = std::collections::hash_map::Keys<'a,
Cid, ()> returning self.inner.keys() so iteration yields keys only.

@codecov
Copy link

codecov bot commented Jan 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.69%. Comparing base (31b5936) to head (8773c2c).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
Files with missing lines Coverage Δ
src/blocks/tipset.rs 87.97% <ø> (+0.50%) ⬆️
src/cid_collections/hash_set.rs 23.07% <ø> (+6.86%) ⬆️

... and 8 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31b5936...8773c2c. Read the comment docs.

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

@LesnyRumcajs LesnyRumcajs added this pull request to the merge queue Jan 26, 2026
Merged via the queue into main with commit 327f9e5 Jan 26, 2026
34 of 35 checks passed
@LesnyRumcajs LesnyRumcajs deleted the derive-into-iterator branch January 26, 2026 17:36
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.

3 participants