Skip to content
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 boost dependency from Dash-specific code #2072

Merged
merged 2 commits into from
Jul 12, 2018

Conversation

nmarley
Copy link

@nmarley nmarley commented May 8, 2018

This is only a partial replacement, as some of the boost items aren't simple swaps. This PR replaces lexical_cast and the boost dependency in CSuperblock shared_ptr. The latter should be straightforward, doesn't use anything specific about boost::shared_ptr that can't be done with std::shared_ptr

@UdjinM6
Copy link

UdjinM6 commented May 10, 2018

Not sure if it's a good idea to touch message signing parts while we are in feature-freeze mode and since this fixes no bug I'd postpone this till 12.4.

@UdjinM6 UdjinM6 added this to the 12.4 milestone May 10, 2018
@nmarley
Copy link
Author

nmarley commented May 10, 2018

I absolutely agree w/you there, my intent was to just get it out for later. I can remember to make a note for PRs that I don't intend to be in the next release, esp. for cases like this where it's in freeze mode.

UdjinM6
UdjinM6 previously approved these changes May 10, 2018
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK for 12.4

nmarley added 2 commits June 30, 2018 19:55
Specifically for spork.cpp : this should be temporary until all spork
sigs are based on hash and not string serialization format, after which
I expect the old signatures (else branch) should go away altogether. But
I still think it's worth it to get pieces of the boost dependency
removed, and this is an easy win, and could be merged now or in a patch
release w/o issue.
@nmarley
Copy link
Author

nmarley commented Jul 4, 2018

Rebased this onto latest develop, so will need a re-ack at this point.

@PastaPastaPasta
Copy link
Member

@nmarley what is the point of removing it from dash-specific code if boost is still needed to compile, due to not changing boost depends from btc?

@nmarley
Copy link
Author

nmarley commented Jul 4, 2018

@paulied So that we can drop it when it's time. It's a big ugly dependency that's largely been superceded with the new C++11/14/17 standards.

Are you aware that Bitcoin is also migrating off Boost? : https://github.com/bitcoin/bitcoin/projects/3

@PastaPastaPasta
Copy link
Member

No I did not see bitcoin was migrating away. This makes a fair bit more sense then. Thanks

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

re-utACK

@UdjinM6 UdjinM6 merged commit 8ee9333 into dashpay:develop Jul 12, 2018
@nmarley nmarley deleted the rm-boost-dash branch August 21, 2018 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants