Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Nov 24, 2020

Now that we can use std::variant from the vanilla standard library, drop the third-party boost variant dependency

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 24, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, 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.

@practicalswift
Copy link
Contributor

Concept ACK

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK.

@maflcko maflcko force-pushed the 2011-noBoostVariant branch from faccd6d to fa06810 Compare November 26, 2020 08:42
@RandyMcMillan
Copy link
Contributor

Concept ACK :)

@theStack
Copy link
Contributor

Concept ACK 🚀

@jonasschnelli
Copy link
Contributor

Concept ACK

1 similar comment
@laanwj
Copy link
Member

laanwj commented Dec 3, 2020

Concept ACK

Copy link
Contributor

@fjahr fjahr left a 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.

maflcko pushed a commit that referenced this pull request Jan 4, 2021
…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
@maflcko maflcko force-pushed the 2011-noBoostVariant branch 4 times, most recently from faad066 to fa5ab4b Compare January 4, 2021 11:24
@maflcko
Copy link
Member Author

maflcko commented Jan 4, 2021

Addressed feedback by @fjahr

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 4, 2021
…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
Copy link
Contributor

@fjahr fjahr left a 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.

@maflcko maflcko force-pushed the 2011-noBoostVariant branch from fa5ab4b to faa8f68 Compare January 5, 2021 09:11
@fjahr
Copy link
Contributor

fjahr commented Jan 5, 2021

Code review ACK faa8f68

Only changes were sorting of includes and switching of CTxDestination from typedef to type alias.

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK faa8f68

@fanquake fanquake merged commit bd6af53 into bitcoin:master Jan 11, 2021
@maflcko maflcko deleted the 2011-noBoostVariant branch January 11, 2021 07:54
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 11, 2021
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Feb 16, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants