Skip to content

Conversation

jferrant
Copy link
Contributor

@jferrant jferrant commented Sep 17, 2025

I am okay with removing InterpreterResult alias throughout the code base entirely but it will be quite a large change. Let me know your thoughts. (will also add some conflicts so might wait until the Error renames are done before doing the full replacement) If we do decide to keep InterpreterResult as VmExecutionResult, I will go throughout the codebase after to update all instances of Result<T, VmExecutionError> to use it as well but want feedback first.

@jferrant jferrant requested review from a team as code owners September 17, 2025 22:29
@jferrant jferrant added the aac Avoiding Accidental Consensus label Sep 17, 2025
@jferrant jferrant linked an issue Sep 17, 2025 that may be closed by this pull request
@fdefelici
Copy link
Contributor

I am okay with removing InterpreterResult alias throughout the code base entirely but it will be quite a large change. Let me know your thoughts. (will also add some conflicts so might wait until the Error renames are done before doing the full replacement) If we do decide to keep InterpreterResult as VmExecutionResult, I will go throughout the codebase after to update all instances of Result<T, VmExecutionError> to use it as well but want feedback first.

I agree! I mean we are not in hurry and we could the less painfull way to remove this alias.
So, I'm fine with doing this activity after the other updates.

… into chore/remove-interpreter-result-as-result
@jcnelson
Copy link
Member

LGTM; happy to re-approve once merge conflicts are resolved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aac Avoiding Accidental Consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove or rename InterpreterResult as Result alias throughout
3 participants