Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate from IntKey to new naked int key #564

Merged
merged 9 commits into from
Dec 1, 2021

Conversation

ueco-jb
Copy link
Contributor

@ueco-jb ueco-jb commented Nov 30, 2021

closes #549

I succesfully replaced every IntKey usage both in contracts and packages (other then implementation itself + tests + de).

Looking at overall progress, I could actually remove whole IntKey implementation along with deserialization.
I wonder if it wouldn't be too drastic? This seems to be major change, although from end user POV nothing changes - it's all internal.

@ueco-jb ueco-jb self-assigned this Nov 30, 2021
@ueco-jb ueco-jb force-pushed the 549-migrate-intkey-to-naked-int-type branch 2 times, most recently from 3a6d5ac to 1bdd257 Compare November 30, 2021 12:59
@ueco-jb ueco-jb marked this pull request as ready for review November 30, 2021 13:11
Copy link
Contributor

@maurolacy maurolacy left a comment

Choose a reason for hiding this comment

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

LGTM. Mostly cosmetic, looks better.

@@ -113,7 +112,7 @@ pub fn execute_register_merkle_root(

let stage = LATEST_STAGE.update(deps.storage, |stage| -> StdResult<_> { Ok(stage + 1) })?;

MERKLE_ROOT.save(deps.storage, U8Key::from(stage), &merkle_root)?;
MERKLE_ROOT.save(deps.storage, stage, &merkle_root)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏼

packages/storage-plus/src/map.rs Outdated Show resolved Hide resolved
@maurolacy
Copy link
Contributor

maurolacy commented Dec 1, 2021

Regarding deprecation / removal of the IntKey types, that would be a breaking change. What we can do is to add a deprecation notice for a while, and then remove, detailing the easy migration path / an example in the docs.

@ueco-jb ueco-jb force-pushed the 549-migrate-intkey-to-naked-int-type branch 2 times, most recently from 69a4dc5 to f432eb9 Compare December 1, 2021 08:33
@ueco-jb ueco-jb force-pushed the 549-migrate-intkey-to-naked-int-type branch from f432eb9 to 237ddee Compare December 1, 2021 08:44
@@ -336,6 +339,7 @@ pub type I128Key = IntKey<i128>;
/// let k = U64Key::new(12345);
/// let k = U32Key::from(12345);
/// let k: U16Key = 12345.into();
#[deprecated(note = "It is suggested to use naked int types instead of IntKey wrapper")]
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏼

@ueco-jb ueco-jb force-pushed the 549-migrate-intkey-to-naked-int-type branch from 9baed5b to 0862505 Compare December 1, 2021 12:07
@ueco-jb ueco-jb merged commit 1e60b0a into main Dec 1, 2021
@ueco-jb ueco-jb deleted the 549-migrate-intkey-to-naked-int-type branch December 1, 2021 12:13
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.

Migrate from IntKey to new naked int key
2 participants