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 code from stacks-core to clar2wasm #367

Merged
merged 13 commits into from
Mar 20, 2024
Merged

Migrate code from stacks-core to clar2wasm #367

merged 13 commits into from
Mar 20, 2024

Conversation

krl
Copy link
Collaborator

@krl krl commented Mar 18, 2024

This PR moves most code related to wasm to the clar2wasm repo.

It seems like where the code should be in the first place, and will make developing changes and tests much less painful.

Draft for now since i need to update the paths to use branches instead of hard-coded paths. This was done to make sure the stacks-core side is still working. A pr in that repo removing the code is coming as well soon.

@Acaccia
Copy link
Collaborator

Acaccia commented Mar 18, 2024

@krl which commit of stacks-core is the code taken from?

@krl
Copy link
Collaborator Author

krl commented Mar 18, 2024

@Acaccia latest feat/clarity-wasm-next

@krl krl marked this pull request as ready for review March 19, 2024 10:44
@krl krl enabled auto-merge March 19, 2024 15:33
Copy link

codecov bot commented Mar 19, 2024

Codecov Report

Attention: Patch coverage is 78.69907% with 298 lines in your changes are missing coverage. Please review.

Project coverage is 87.15%. Comparing base (4abad1a) to head (2d95b17).

Files Patch % Lines
clar2wasm/src/wasm_utils.rs 73.68% 154 Missing and 131 partials ⚠️
clar2wasm/src/initialize.rs 95.88% 2 Missing and 11 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #367      +/-   ##
==========================================
- Coverage   91.14%   87.15%   -4.00%     
==========================================
  Files          40       43       +3     
  Lines       14063    18510    +4447     
  Branches    14063    18510    +4447     
==========================================
+ Hits        12818    16132    +3314     
- Misses        573     1048     +475     
- Partials      672     1330     +658     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@krl krl requested a review from Acaccia March 19, 2024 15:52
Copy link
Collaborator

@csgui csgui left a comment

Choose a reason for hiding this comment

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

From what I got, code that was on stacks-core was pushed to clarity-wasm repo and split into initialize, wasm_utils and linker files.

With that change, initialize is now the orchestrator when dealing with Clarity contracts initialization and execution on wasm.

All set os tests are running ok (only one regression already noted).

Code LGTM! Approved. But would be great to also have another pair of eyes on those changes. /cc @Acaccia

Good work @krl ! Thanks!

@krl krl added this pull request to the merge queue Mar 20, 2024
Merged via the queue into main with commit 9533a5c Mar 20, 2024
6 of 8 checks passed
@krl krl deleted the code-migration branch March 20, 2024 11:37
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.

None yet

3 participants