Page MenuHomePhabricator

[avalanche] Leverage AnyVoteItem to have a single VoteItemUpdate object
ClosedPublic

Authored by Fabien on Jan 18 2023, 15:43.

Details

Summary

Remove the type specific vote update, so a single VoteItemUpdate can be used and remove some boilerplate.

There is no change in behavior.

Depends on D12995, D12996, D12997.

Test Plan
ninja all check-all

Diff Detail

Event Timeline

Fabien requested review of this revision.Jan 18 2023, 15:43
sdulfari requested changes to this revision.Jan 18 2023, 17:49
sdulfari added a subscriber: sdulfari.
sdulfari added inline comments.
src/avalanche/test/processor_tests.cpp
283

Why not getItem?

370–375

This test case is now item type agnostic.

This revision now requires changes to proceed.Jan 18 2023, 17:49
src/avalanche/test/processor_tests.cpp
283

To make it more explicit, especially when you read the test case you might not understand why you need an additional call to getItem (you already have an item, right ?), despite fromAnyVoteItem make it clear that you have the wrong type. Let me know if you disagree.

370–375

ah good catch, I initially unrolled but forgot to rename after I switched to a templated test case

Rename the test variable

sdulfari added inline comments.
src/avalanche/test/processor_tests.cpp
283

I think there's something missing because the "from" implies a "to" which doesn't read well in the tests or anywhere else. But I don't want to block on this so this patch is green. We can discuss offline and rename if something else is better.

This revision is now accepted and ready to land.Jan 20 2023, 16:39