Checkpoints misc cleanup #6242
Checkpoints misc cleanup #6242
Conversation
src/chainparams.cpp
Outdated
There was a problem hiding this comment.
I vaguely remember that we already went through this change once, but the other way around. The point to use boost here is that it easily transitions to c++11 lingo. I'd like to leave this alone until we do.
There was a problem hiding this comment.
Here,
- only two years ago this was all changed to list_of in the name of newness: 8388289
- Then you changed it to get rid of the global namespace 856e862
- Then some list_ofs had to be changed to std::vector again a2b04dd in the name of c++11
... and now we're full circle, back to C-style initialization. Sorry for this, this may be just a personal annoyance against flip-flopping perfectly fine code, but I feel this is not helping anyone :)
There was a problem hiding this comment.
Well in fairness, the last two steps reduced the boost dependency, this just goes the final step and actually removes it. I agree that the gradual touching and "improving" can get annoying, but I don't think there's been any flip-flopping here, just small steps.
I understand if you'd rather hold on this and use initializer lists instead, but I may pick up this fight again if this would be usable in a lib before we move to c++11 :)
Edit: whoops, dangling text removed.
There was a problem hiding this comment.
I was also removing boost dependencies from consensus code, but now I am not sure what is preferred. Chainparams is out of consensus and it seems reasonable to move towards c++11.
But maybe we want to eventually have a libconsensus not only exposed but also written in C?
In any case, I would really like to have a more clear criterion.
My idea was "remove boost when possible", then @laanwj told be that boost/foreach was there for later c++11, and I thought that was an exception.
But now I'm really confused and I definitely want to avoid going in circles.
|
utACK for me, with or without deboostification, but needs rebase. |
|
Yes, untested ACK apart from the boost discussion. |
|
Still needs rebase. |
This unties CChainParams from its dependency on checkpoints. Instead, now it only depends on the raw checkpoint data.
6244dde to
17221bf
Compare
|
Rebased and dropped the boost change. |
|
re-utACK. |
|
ACK |
There was a problem hiding this comment.
It is true that it doesn't add much readability at this point. Maybe you can fix it with a commit to the trivial branch?
There was a problem hiding this comment.
@theuni would need to rebase said branch to include this.
Bitcoin 0.12 chainparams cleanups Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6222 - bitcoin/bitcoin#6381 - bitcoin/bitcoin#6473 - bitcoin/bitcoin#6242 Part of #2074.
Bitcoin 0.12 chainparams cleanups Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6222 - bitcoin/bitcoin#6381 - bitcoin/bitcoin#6473 - bitcoin/bitcoin#6242 Part of #2074.
This is part of a much larger networking refactor that I'm working on, but it broke out cleanly so I'm submitting it independently.
Chainparams will soon be free of app state, so these are a few easy cleanups towards that end. Specifically, this drops the dependencies on checkpoints (and main as a side-effect) and boost.