Move CBlockIndexWorkComparator to its own header
ClosedPublic
Actions

Authored by jasonbcox on Jul 4 2018, 00:28.

Details

Reviewers

deadalnix
schancel

Group Reviewers

Restricted Project

Commits

rSTAGING14893d239ab6: Move CBlockIndexWorkComparator to its own header
rABC14893d239ab6: Move CBlockIndexWorkComparator to its own header

Summary

CBlockIndexWorkComparator belongs in its own header rather than validation.cpp. Also, this makes it more accessible for writing tests.

Test Plan

make check

Diff Detail

Repository

rABC Bitcoin ABC

Branch

comp

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 2796
Build 3702: Bitcoin ABC Buildbot (legacy)
Build 3701: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Jul 4 2018, 00:28

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 4 2018, 00:28

Build Bitcoin-ABC :: Diffs :: Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC :: Diffs :: Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B2796: Diff 4197.Jul 4 2018, 01:05

Can you explain why the is moving into an header ? Probably belongs here is not a very good reason. If somethign is used only in one place, it belong there. Writing a test is a good reason to do it, but right now there are no tests for it.

Ok so we discussed this offline.

The benefit of putting this in a header is obviously to be able to write tests for it, which is great. The downside is that changing it will require to recompile half of the world due to chain.h being a very common header. A good way to proceed here would be to create a new header for CBlockIndexWorkComparator and include that header in validation.cpp instead of moving it into chain.h .

Moved to its own header

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

jasonbcox retitled this revision from Move CBlockIndexWorkComparator to chain.h to Move CBlockIndexWorkComparator to its own header.Jul 10 2018, 17:54

jasonbcox edited the summary of this revision. (Show Details)

Harbormaster completed remote builds in B2838: Diff 4259.Jul 10 2018, 18:29

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

jasonbcox added a child revision: D1555: Added test for CBlockIndexWorkComparator.Jul 10 2018, 18:42

deadalnix accepted this revision.Jul 10 2018, 23:07

This revision is now accepted and ready to land.Jul 10 2018, 23:07

Closed by commit rABC14893d239ab6: Move CBlockIndexWorkComparator to its own header (authored by jasonbcox). · Explain WhyJul 11 2018, 17:06

This revision was automatically updated to reflect the committed changes.