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

Refactor creating new StorableSlab #324

Conversation

fxamacker
Copy link
Member

@fxamacker fxamacker commented Jul 6, 2023

Updates #296 #292 onflow/flow-go#1744

Description

Currently, in order to create StorableSlab, user needs to:

  • generate SlabID (used to call StorageID) from storage
  • create new StorableSlab
  • store created StorableSlab in storage

This commit unexports StorableSlab fields and adds NewStorableSlab().

With this change, user can simply call NewStorableSlab().


  • Targeted PR against main branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

Currently, in order to create StorableSlab, user needs to:
- generate SlabID from storage
- create new StorableSlab
- store created StorableSlab in storage

This commit unexports StorableSlab fields and adds NewStorableSlab().
With this change, user can simply call NewStorableSlab().
@fxamacker fxamacker added improvement breaking change changes to API that can break programs using Atree's API labels Jul 6, 2023
@fxamacker fxamacker self-assigned this Jul 6, 2023
ValueID identifies atree Array and OrderedMap while SlabID identifies
slab in storage.  SlabID should only be used to retrieve, store, and
remove slab in storage.
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

👌

@codecov-commenter
Copy link

codecov-commenter commented Jul 6, 2023

Codecov Report

Merging #324 (37f2201) into fxamacker/unexport-SlabID-fields (33b8013) will decrease coverage by 0.12%.
The diff coverage is 38.77%.

@@                         Coverage Diff                          @@
##           fxamacker/unexport-SlabID-fields     #324      +/-   ##
====================================================================
- Coverage                             64.64%   64.53%   -0.12%     
====================================================================
  Files                                    14       14              
  Lines                                  8050     8075      +25     
====================================================================
+ Hits                                   5204     5211       +7     
- Misses                                 2168     2187      +19     
+ Partials                                678      677       -1     
Impacted Files Coverage Δ
slab.go 100.00% <ø> (ø)
storage.go 74.48% <ø> (+0.31%) ⬆️
storable_slab.go 31.88% <25.00%> (-13.58%) ⬇️
array.go 69.92% <100.00%> (ø)
array_debug.go 52.28% <100.00%> (ø)
encode.go 79.36% <100.00%> (ø)
map.go 66.88% <100.00%> (+0.09%) ⬆️
map_debug.go 51.77% <100.00%> (ø)

@fxamacker fxamacker merged commit 5224515 into fxamacker/unexport-SlabID-fields Jul 6, 2023
fxamacker added a commit to onflow/cadence that referenced this pull request Jul 7, 2023
This commit updates Cadence to use new Atree API
- Array.SlabID()
- OrderedMap.SlabID()
- SlabID
- SlabIndex
- etc.

For more info, see Atree PRs:
- onflow/atree#322
- onflow/atree#323
- onflow/atree#324
fxamacker added a commit to onflow/cadence that referenced this pull request Jul 7, 2023
This commit updates Cadence to use new Atree API
- Array.SlabID()
- OrderedMap.SlabID()
- SlabID
- SlabIndex
- etc.

For more info, see Atree PRs:
- onflow/atree#322
- onflow/atree#323
- onflow/atree#324
@fxamacker fxamacker mentioned this pull request Jul 7, 2023
6 tasks
turbolent pushed a commit to onflow/cadence that referenced this pull request Jan 26, 2024
This commit updates Cadence to use new Atree API
- Array.SlabID()
- OrderedMap.SlabID()
- SlabID
- SlabIndex
- etc.

For more info, see Atree PRs:
- onflow/atree#322
- onflow/atree#323
- onflow/atree#324
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change changes to API that can break programs using Atree's API improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants