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

Combine storage traits and derives into single crate #1389

Merged
merged 25 commits into from
Sep 13, 2022

Conversation

ascjones
Copy link
Collaborator

@ascjones ascjones commented Sep 8, 2022

Follow up to #1331. Reviewers don't be alarmed this is just moving a bunch of stuff around.

This PR:

  • Creates new ink_storage_traits crate
  • Moves Storable definition and derive there (together with other traits)
  • Moves some types to primitives to remove dependency on ink_env

It is to make life easier for storage derive macro crate identifiers in #1223.

  • Currently ink_primitives contains the Storable trait definition and derive.
    • This is not a natural place for it, just required to avoid circular depenency (see below)
  • ink_storage -> ink_env, so ink_env cannot depend on ink_storage (circular dependency)

@ascjones ascjones marked this pull request as ready for review September 9, 2022 11:32
@ascjones ascjones requested review from a team, cmichi and HCastano as code owners September 9, 2022 11:32
@codecov-commenter
Copy link

codecov-commenter commented Sep 9, 2022

Codecov Report

Merging #1389 (c0956e4) into master (c0956e4) will not change coverage.
The diff coverage is n/a.

❗ Current head c0956e4 differs from pull request most recent head 1dc778b. Consider uploading reports for the commit 1dc778b to get more accurate results

@@           Coverage Diff           @@
##           master    #1389   +/-   ##
=======================================
  Coverage   71.42%   71.42%           
=======================================
  Files         190      190           
  Lines        5873     5873           
=======================================
  Hits         4195     4195           
  Misses       1678     1678           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@xgreenx xgreenx left a comment

Choose a reason for hiding this comment

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

It looks clearly now=)
I like that Storable in the right place right now. And I definitely like definition of the AccountId, Hash in the ink_ptimitives=)

@ascjones ascjones merged commit fb871a0 into master Sep 13, 2022
@ascjones ascjones deleted the aj/storage-traits-crate branch September 13, 2022 10:39
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.

4 participants