-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Replace boost::variant with std::variant #20480
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
Conversation
fabb2ab
to
faccd6d
Compare
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
Concept ACK |
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.
Concept ACK.
faccd6d
to
fa06810
Compare
Concept ACK :) |
Concept ACK 🚀 |
Concept ACK |
1 similar comment
Concept ACK |
fa06810
to
fa1b3b6
Compare
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.
Concept ACK
Code looks good, I had started looking into similar changes before I found this.
…g.m_fallback fa749fb rpc: Replace boost::variant with std::variant for RPCArg.m_fallback (MarcoFalke) Pull request description: Now that we can use std::variant from the vanilla standard library, drop the third-party boost variant dependency. Patch is split out from #20480. A step-by-step replacement is possible because we don't have our own `Variant` wrapper and the source code specifies `boost::variant` explicitly. I think a step-by-step replacement should be preferred, because it simplifies review. ACKs for top commit: fjahr: re-ACK fa749fb Sjors: re-ACK fa749fb Tree-SHA512: 5e3c12b7d535f73065b4afa8df0a488f78fb25d2234f5ecbf740e624db03a34c35fea100eb7d37e84741721310e6450b7fb4296a2207a7ed1fa24485b3650981
faad066
to
fa5ab4b
Compare
Addressed feedback by @fjahr |
…r RPCArg.m_fallback fa749fb rpc: Replace boost::variant with std::variant for RPCArg.m_fallback (MarcoFalke) Pull request description: Now that we can use std::variant from the vanilla standard library, drop the third-party boost variant dependency. Patch is split out from bitcoin#20480. A step-by-step replacement is possible because we don't have our own `Variant` wrapper and the source code specifies `boost::variant` explicitly. I think a step-by-step replacement should be preferred, because it simplifies review. ACKs for top commit: fjahr: re-ACK fa749fb Sjors: re-ACK fa749fb Tree-SHA512: 5e3c12b7d535f73065b4afa8df0a488f78fb25d2234f5ecbf740e624db03a34c35fea100eb7d37e84741721310e6450b7fb4296a2207a7ed1fa24485b3650981
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.
Code review ACK fa5ab4b
Thanks, the simplifications made this very straight forward to review IMO.
fa5ab4b
to
faa8f68
Compare
Code review ACK faa8f68 Only changes were sorting of includes and switching of |
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.
ACK faa8f68
Now that we can use std::variant from the vanilla standard library, drop the third-party boost variant dependency