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

vm: import construction → respective VM impls #11506

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

nagisa
Copy link
Collaborator

@nagisa nagisa commented Jun 6, 2024

I always found it weird that we had VM specific code in what's a generic
part of the near-vm-runner. Well -- it has actually started bothering me
in real ways, so it gets moved.

I didn't do very much to make sure it ends up pretty. And I don't think
I want to for the VMs that're not wasmtime or near_vm. Good thing is
that the wasmer0/2 code can be largely ignored sans changes to VMLogic
(which I'm considering addressing next...)

Based on top of #11503
Part of #11319

@nagisa nagisa requested a review from a team as a code owner June 6, 2024 12:10
@nagisa nagisa requested a review from tayfunelmas June 6, 2024 12:10
Copy link

codecov bot commented Jun 6, 2024

Codecov Report

Attention: Patch coverage is 96.02649% with 12 lines in your changes missing coverage. Please review.

Project coverage is 71.37%. Comparing base (fe5a71c) to head (eb02806).

Files Patch % Lines
...untime/near-vm-runner/src/near_vm_runner/runner.rs 95.91% 0 Missing and 4 partials ⚠️
runtime/near-vm-runner/src/wasmer2_runner.rs 96.11% 0 Missing and 4 partials ⚠️
runtime/near-vm-runner/src/wasmtime_runner.rs 93.22% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11506      +/-   ##
==========================================
- Coverage   71.37%   71.37%   -0.01%     
==========================================
  Files         790      790              
  Lines      160086   160087       +1     
  Branches   160086   160087       +1     
==========================================
  Hits       114256   114256              
+ Misses      40857    40853       -4     
- Partials     4973     4978       +5     
Flag Coverage Δ
backward-compatibility 0.23% <0.00%> (ø)
db-migration 0.23% <0.00%> (ø)
genesis-check 1.35% <0.00%> (+<0.01%) ⬆️
integration-tests 37.65% <61.58%> (-0.02%) ⬇️
linux 68.80% <96.02%> (+0.02%) ⬆️
linux-nightly 70.90% <96.02%> (-0.01%) ⬇️
macos 52.42% <93.22%> (-0.01%) ⬇️
pytests 1.57% <0.00%> (+<0.01%) ⬆️
sanity-checks 1.37% <0.00%> (+<0.01%) ⬆️
unittests 66.07% <96.02%> (-0.01%) ⬇️
upgradability 0.28% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@nagisa nagisa force-pushed the moves-around-import-code branch 2 times, most recently from d98ecef to 204ec83 Compare June 7, 2024 12:14
@nagisa nagisa enabled auto-merge June 7, 2024 12:14
@nagisa nagisa disabled auto-merge June 7, 2024 12:14
I always found it weird that we had VM specific code in what's a generic
part of the near-vm-runner. Well -- it has actually started bothering me
in real ways, so it gets moved.

I didn't do very much to make sure it ends up pretty. And I don't think
I want to for the VMs that're not wasmtime or near_vm. Good thing is
that the wasmer0/2 code can be largely ignored sans changes to `VMLogic`
(which I'm considering addressing next...)
@nagisa nagisa force-pushed the moves-around-import-code branch from 204ec83 to eb02806 Compare June 10, 2024 11:14
@nagisa nagisa enabled auto-merge June 10, 2024 11:14
@nagisa nagisa added this pull request to the merge queue Jun 10, 2024
Merged via the queue into near:master with commit 75eee5e Jun 10, 2024
28 of 29 checks passed
@nagisa nagisa deleted the moves-around-import-code branch June 10, 2024 11:50
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.

2 participants