Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

runtime: remove Default req on account scan interfaces #28533

Merged
merged 2 commits into from
Oct 21, 2022

Conversation

t-nelson
Copy link
Contributor

Problem

the accounts scanning interfaces have a seemingly unnecessary Default requirement, which restricts what types can be used to collect in to.

Summary of Changes

switch to FnMut closures and move the accumulator construction out to the call sites

@t-nelson
Copy link
Contributor Author

first commit is the interesting one. second is straightforward from there

@Lichtso
Copy link
Contributor

Lichtso commented Oct 21, 2022

Might be more effort than it is worth, but can't scan_accounts() return an impl Iterator? Then we could simply collect into a container at the call site.

jeffwashington
jeffwashington previously approved these changes Oct 21, 2022
Copy link
Contributor

@jeffwashington jeffwashington left a comment

Choose a reason for hiding this comment

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

lgtm

@mergify mergify bot dismissed jeffwashington’s stale review October 21, 2022 21:23

Pull request has been modified.

@t-nelson
Copy link
Contributor Author

Might be more effort than it is worth, but can't scan_accounts() return an impl Iterator? Then we could simply collect into a container at the call site.

i'd considered that but a couple of its sibling methods have some extra logic and i opted for a consistent api across them instead

@t-nelson t-nelson merged commit 1fbd818 into solana-labs:master Oct 21, 2022
@t-nelson t-nelson deleted the sand branch October 21, 2022 23:53
t-nelson added a commit to t-nelson/solana that referenced this pull request Oct 26, 2022
t-nelson added a commit to t-nelson/solana that referenced this pull request Oct 26, 2022
t-nelson added a commit to t-nelson/solana that referenced this pull request Oct 26, 2022
gnapoli23 pushed a commit to gnapoli23/solana that referenced this pull request Dec 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants