Conversation
|
Concept ACK |
|
This overlaps with #10193. However given it's a smaller change-set, and not solely about nuking Boost, maybe we can merge it first? Depends on how many other nearly/ready-to-merge PRs it's going to break. |
ryanofsky
left a comment
ryanofsky
left a comment
There was a problem hiding this comment.
utACK 2ffa099308e2f3566531e30e5754f83800adbd7d. Nice change. I think it would be good to merge this before #10193 because this change is more targeted and will only make that PR easier to review. I would expect that rebasing #10193 should not be very difficult because most of the commits are scripted.
src/utilstrencodings.h
Outdated
There was a problem hiding this comment.
Bizarre that pairtype was defined in utilstrencodings.h.
src/wallet/wallet.cpp
Outdated
src/wallet/wallet.cpp
Outdated
There was a problem hiding this comment.
I think this can be const auto.
src/wallet/wallet.cpp
Outdated
There was a problem hiding this comment.
I think this can be const.
src/wallet/wallet.cpp
Outdated
There was a problem hiding this comment.
This is preserving behavior, but copying CWalletTx isn't ideal. I made a change in #10500 to prevent this.
|
@ryanofsky thanks, I addressed all of your feedback. |
ryanofsky
left a comment
ryanofsky
left a comment
There was a problem hiding this comment.
utACK 6b50bb066a55d073f870afb12ab57482da725735
|
Rebasing #10193 isn't hard at all, once I wrote the sed scripts once it's just running them again. The problem with #10193 is that the reverse iterator to replace BOOST_REVERSE_FOREACH isn't working for some reason. So I separated #10502 with the first 3 commits that could be merged right away. |
|
Needs rebase |
Replace all BOOST_FOREACH and Q_FOREACH loops using it with range based loops.
|
@MarcoFalke rebased. |
|
utACK b348957 Very nice! |
|
Can you write this as a scripted-diff? |
Can't we just merge #10502 instead? |
|
@jtimon Cool! |
|
As I said before, I don't mind at all to merge #10502 first. Then I don't have to repeat the effort of scripting the diff and after rebase this will simply be removing the macro. |
|
The only difference is that this one puts `auto` in place of
`std::pair<type1, type2>`.
…On Tue, Jun 13, 2017 at 8:35 PM, Wladimir J. van der Laan < ***@***.***> wrote:
Can't we just merge #10502 <#10502>
instead?
So: close this in favor of #10502
<#10502>, as it encompasses more
and is scripted?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#10497 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGGmv-c-ROfD9_DFX9h4m3ojKwxHtCiUks5sDtZegaJpZM4Ns5Uv>
.
|
|
I think when this PR was started, the other one didn't deal with the Q_FOREACH and removing the pairtype macro (or I was blind ;)). Now that it does, I'll close this PR. (I don't mind the explicit std::pair annotation). |
|
#10193 didn't remove PAIRTYPE until @tjps suggested it in #10193 (comment) |
9 participants
Replace all BOOST_FOREACH and Q_FOREACH loops using it with range
based loops.