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

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

Details

Reviewers

sdulfari

Group Reviewers

Restricted Project

Commits

Summary

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

There is no change in behavior.

Test Plan

ninja all check-all

Repository

rABC Bitcoin ABC

Branch

avalanche_voteitemupdate

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 21781
Build 43197: Build Diff	lint-circular-dependencies · build-debug · build-without-wallet · build-diff · build-clang · build-clang-tidy
Build 43196: arc lint + arc unit

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

This revision was automatically updated to reflect the committed changes.