Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

add into function for PrefixIterator #10614

Merged
merged 3 commits into from
Jan 9, 2022
Merged

add into function for PrefixIterator #10614

merged 3 commits into from
Jan 9, 2022

Conversation

Mr-Leshiy
Copy link
Contributor

Small refactor, add into function which conveerts PrefixIterator<T, O1> into another PrefixIterator<T, O2>

kianenigma
kianenigma suggested changes Jan 8, 2022
View reviewed changes
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Not a fan of calling normal functions same as common traits. If this has to be done as a free functions, rather call it into_other or something else.

Also,

Can't this be done with actual From or Into? e.g. Impl<T, OR1, OR2> From<PrefixIterator<T, OR1>> for From<PrefixIterator<T, OR2>

@Mr-Leshiy
Copy link
Contributor Author

@kianenigma , Unfortunately it is not possible to implement From or Into, we have the similar situation as described here https://users.rust-lang.org/t/conflicting-implementations-for-trait-core-convert-from/3149.

So ok, will rename it to into_other.

@Mr-Leshiy Mr-Leshiy requested a review from kianenigma January 8, 2022 17:08
kianenigma
kianenigma reviewed Jan 9, 2022
View reviewed changes
frame/support/src/storage/mod.rs Outdated
@@ -802,6 +802,18 @@ pub struct PrefixIterator<T, OnRemoval = ()> {
phantom: core::marker::PhantomData<OnRemoval>,
}

impl<T, OnRemoval1> PrefixIterator<T, OnRemoval1> {
pub fn into_other<OnRemoval2>(self) -> PrefixIterator<T, OnRemoval2> {
Copy link
Contributor

Choose a reason for hiding this comment

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

pub item needs documentation.

Copy link
Member

Choose a reason for hiding this comment

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

And I would call this convert_on_removal. into_other isn't really expressing.

@Mr-Leshiy Mr-Leshiy requested a review from bkchr January 9, 2022 09:48
@bkchr bkchr added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Jan 9, 2022
@bkchr bkchr merged commit c623d17 into paritytech:master Jan 9, 2022
@Mr-Leshiy Mr-Leshiy deleted the refactor-prefix-iterator branch January 10, 2022 08:14
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
@Mr-Leshiy @grishasobol
add into function for PrefixIterator (paritytech#10614)
f20299b 
* add into function for PrefixIterator

* update with comments

* update with comments
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
@Mr-Leshiy @ark0f
add into function for PrefixIterator (paritytech#10614)
6f72f6c 
* add into function for PrefixIterator

* update with comments

* update with comments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr approved these changes

@kianenigma kianenigma kianenigma approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Mr-Leshiy @kianenigma @bkchr