Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Inline verifier struct creation #3252

Merged
merged 1 commit into from
Jul 31, 2024
Merged

Inline verifier struct creation #3252

merged 1 commit into from
Jul 31, 2024

Conversation

dhrubabasu
Copy link
Contributor

Why this should be merged

Required for #3236

We'll need the proposerVMCtx to be passed into verifier which is nicer to construct on the fly.

How this works

Inline verifier struct creation

How this was tested

CI

@dhrubabasu dhrubabasu added cleanup Code quality improvement acp77 labels Jul 30, 2024
@dhrubabasu dhrubabasu added this to the v1.11.11 milestone Jul 30, 2024
@dhrubabasu dhrubabasu self-assigned this Jul 30, 2024
Copy link
Contributor

@tsachiherman tsachiherman left a comment

Choose a reason for hiding this comment

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

the only downside I can see here is the fact that if we were to have a dozen of places where verifier is being constructed in that way and we add another member to verifier, than we might miss adding that field to all the instances.

@marun
Copy link
Contributor

marun commented Jul 30, 2024

the only downside I can see here is the fact that if we were to have a dozen of places where verifier is being constructed in that way and we add another member to verifier, than we might miss adding that field to all the instances.

This might be a good opportunity to invoke YAGNI, since the type is currently only constructed in a single place (outside of testing) and is not exported.

@StephenButtolph StephenButtolph added this pull request to the merge queue Jul 31, 2024
Merged via the queue into master with commit e327ddb Jul 31, 2024
20 checks passed
@StephenButtolph StephenButtolph deleted the inline-verifier branch July 31, 2024 13:49
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
acp77 cleanup Code quality improvement
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants