Page MenuHomePhabricator

[rpc] Move tojson.h into blockchain.h
ClosedPublic

Authored by markblundeberg on May 10 2019, 05:13.

Details

Summary

In order to make backports go smoothly. (file was introduced in D717)

Test Plan

compile

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

note: TxToJSON is not in blockchain.cpp and doesn't need to be in a header either

schancel requested changes to this revision.May 12 2019, 23:47

I think these should be left in place.

This revision now requires changes to proceed.May 12 2019, 23:47
src/rpc/tojson.h
14 ↗(On Diff #8582)

If you're going to delete this from the header, you should mark its implementation as static in rawtransaction.cpp

I think these should be left in place.

Why?

I think more and more stuff should be better organized. Core doesn't seem to care about separating stuff out much.

However, this is just my opinion. It seems Amaury shared it. I hope we don't prevent ourself from making needed organizational changes indefinitely because of backports.

I think more and more stuff should be better organized. Core doesn't seem to care about separating stuff out much.

However, this is just my opinion. It seems Amaury shared it. I hope we don't prevent ourself from making needed organizational changes indefinitely because of backports.

Ah I see. Well the original organization was definitely crap, as both Core and Amaury recognized. I guess my mental picture here is that if the backport of PR10095 had already been done, then the tojson.h file would never have been made in the first place and a divergence would not have occurred. I also personally think the blockchain.h is the right place for these functions (being defined in blockchain.cpp).

Okay, I see. That's weird -- still not sure *toJSON functions should go there, but okay.

This revision is now accepted and ready to land.May 13 2019, 18:45

Okay, I see. That's weird -- still not sure *toJSON functions should go there, but okay.

Yeah, as I was typing that I was thinking "why are they in blockchain.cpp anyway?".. Well, once these functions are caught up we can look at messing around with them.