-
Notifications
You must be signed in to change notification settings - Fork 38.7k
remove the PAIRTYPE macro #10497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove the PAIRTYPE macro #10497
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bizarre that pairtype was defined in utilstrencodings.h.
src/wallet/wallet.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Space missing after for
src/wallet/wallet.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be const auto.
src/wallet/wallet.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be const.
src/wallet/wallet.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) |
Replace all BOOST_FOREACH and Q_FOREACH loops using it with range
based loops.