Skip to content

Utils and libraries: Removes the boost/algorithm/string/join dependency #13726

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 22, 2018
Merged

Utils and libraries: Removes the boost/algorithm/string/join dependency #13726

merged 1 commit into from
Jul 22, 2018

Conversation

l2a5b1
Copy link
Contributor

@l2a5b1 l2a5b1 commented Jul 20, 2018 •
edited
Loading

This commit removes the boost/algorithm/string/join dependency from the project by replacing boost::algorithm::join with the helper function proposed by @MarcoFalke in #13726 (comment)

@l2a5b1 l2a5b1 force-pushed the patch/remove_boost_join_dependency branch from 2732d07 to c3f1970 Compare July 20, 2018 19:03
@Empact
Copy link
Contributor

Empact commented Jul 20, 2018 •
edited
Loading

Given it's only used in the one place, I'd be inclined to make it static to the file used, or making a more narrow util file for it.

I like Marco's idea.

maflcko
maflcko reviewed Jul 20, 2018
View reviewed changes
src/utilstrencodings.h Outdated
}
return stream.str();
}

Copy link
Member

Choose a reason for hiding this comment

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

Instead of a whole util function you might just inline a helper to append the warning:

static void AppendWarn(std::string& res, const std::string& warn){
if (!res.empty()) res+=", ";
res+=warn;
}

@l2a5b1 l2a5b1 force-pushed the patch/remove_boost_join_dependency branch 3 times, most recently from 03a048b to f94ec7c Compare July 20, 2018 21:56
@l2a5b1
Copy link
Contributor Author

l2a5b1 commented Jul 20, 2018

Thanks @MarcoFalke and @Empact, appreciate the feedback!

I have implemented the proposed helper function in f94ec7c and updated the pull request description accordingly.

Empact
Empact reviewed Jul 20, 2018
View reviewed changes
src/validation.cpp Outdated
@@ -2293,7 +2299,7 @@ void static UpdateTip(const CBlockIndex *pindexNew, const CChainParams& chainPar
FormatISO8601DateTime(pindexNew->GetBlockTime()),
GuessVerificationProgress(chainParams.TxData(), pindexNew), pcoinsTip->DynamicMemoryUsage() * (1.0 / (1<<20)), pcoinsTip->GetCacheSize());
if (!warningMessages.empty())
LogPrintf(" warning='%s'", boost::algorithm::join(warningMessages, ", ")); /* Continued */
LogPrintf(" warning='%s'", warningMessages.c_str()); /* Continued */
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think c_str is needed here.

@l2a5b1
Removes the boost/algorithm/string/join dependency
5f019d5 
This commit removes the `boost/algorithm/string/join` dependency
from the project by replacing `boost::algorithm::join` with
a simple helper function.
@l2a5b1 l2a5b1 force-pushed the patch/remove_boost_join_dependency branch from f94ec7c to 5f019d5 Compare July 20, 2018 23:15
@Empact
Copy link
Contributor

Empact commented Jul 20, 2018

utACK 5f019d5

@l2a5b1
Copy link
Contributor Author

l2a5b1 commented Jul 20, 2018

Thanks @Empact, yeah your right. I didn't expect that one 👍
Addressed in 5f019d5.

@practicalswift
Copy link
Contributor

utACK 5f019d5

Thanks for helping remove our Boost dependencies! Please continue :-)

@sipa
Copy link
Member

sipa commented Jul 21, 2018

utACK 5f019d5

@maflcko
Copy link
Member

maflcko commented Jul 22, 2018

utACK 5f019d5

@maflcko maflcko merged commit 5f019d5 into bitcoin:master Jul 22, 2018
maflcko pushed a commit that referenced this pull request Jul 22, 2018
Merge #13726: Utils and libraries: Removes the boost/algorithm/string…
eeae471 
…/join dependency

5f019d5 Removes the boost/algorithm/string/join dependency (251)

Pull request description:

  This commit removes the `boost/algorithm/string/join` dependency from the project by replacing `boost::algorithm::join` with the helper function proposed by @MarcoFalke in #13726 (comment)

Tree-SHA512: d4ba3e7621b76bd5210aec9b8d6c320f7ee963d7f902e6d2d3fc0eadbee1cd77799e5c09be9c11452d2825f25740fc436cdec3a6b6c66ced674d771e4ed306ae
@Bushstar Bushstar mentioned this pull request Jul 24, 2018
Merged
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jun 28, 2019
@markblundeberg
Merge #13726: Utils and libraries: Removes the boost/algorithm/string…
0b45fa8 
…/join dependency

Summary:
PR13726 backport https://github.com/bitcoin/bitcoin/pull/13726/files
5f019d5 Removes the boost/algorithm/string/join dependency (251)

Pull request description:

  This commit removes the `boost/algorithm/string/join` dependency from the project by replacing `boost::algorithm::join` with the helper function proposed by @MarcoFalke in bitcoin/bitcoin#13726 (comment)

Backporter notes:
- One line missing due to prior removal of versionbits logic.

Test Plan: `make check`

Reviewers: jasonbcox, Fabien, schancel, #bitcoin_abc, deadalnix

Reviewed By: jasonbcox, Fabien, #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D3381
jtoomim pushed a commit to jtoomim/bitcoin-abc that referenced this pull request Jun 29, 2019
@jtoomim
Merge #13726: Utils and libraries: Removes the boost/algorithm/string…
d265340 
…/join dependency

Summary:
PR13726 backport https://github.com/bitcoin/bitcoin/pull/13726/files
5f019d5 Removes the boost/algorithm/string/join dependency (251)

Pull request description:

  This commit removes the `boost/algorithm/string/join` dependency from the project by replacing `boost::algorithm::join` with the helper function proposed by @MarcoFalke in bitcoin/bitcoin#13726 (comment)

Backporter notes:
- One line missing due to prior removal of versionbits logic.

Test Plan: `make check`

Reviewers: jasonbcox, Fabien, schancel, #bitcoin_abc, deadalnix

Reviewed By: jasonbcox, Fabien, #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D3381
@Empact Empact mentioned this pull request Oct 17, 2019
Merged
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 19, 2020
@PastaPastaPasta
Merge bitcoin#13726: Utils and libraries: Removes the boost/algorithm…
fb76eea 
…/string/join dependency

5f019d5 Removes the boost/algorithm/string/join dependency (251)

Pull request description:

  This commit removes the `boost/algorithm/string/join` dependency from the project by replacing `boost::algorithm::join` with the helper function proposed by @MarcoFalke in bitcoin#13726 (comment)

Tree-SHA512: d4ba3e7621b76bd5210aec9b8d6c320f7ee963d7f902e6d2d3fc0eadbee1cd77799e5c09be9c11452d2825f25740fc436cdec3a6b6c66ced674d771e4ed306ae
Signed-off-by: pasta <pasta@dashboost.org>

# Conflicts:
#	contrib/devtools/lint-includes.sh
#	src/validation.cpp
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 24, 2020
@PastaPastaPasta
Merge bitcoin#13726: Utils and libraries: Removes the boost/algorithm…
e797b40 
…/string/join dependency

5f019d5 Removes the boost/algorithm/string/join dependency (251)

Pull request description:

  This commit removes the `boost/algorithm/string/join` dependency from the project by replacing `boost::algorithm::join` with the helper function proposed by @MarcoFalke in bitcoin#13726 (comment)

Tree-SHA512: d4ba3e7621b76bd5210aec9b8d6c320f7ee963d7f902e6d2d3fc0eadbee1cd77799e5c09be9c11452d2825f25740fc436cdec3a6b6c66ced674d771e4ed306ae
Signed-off-by: pasta <pasta@dashboost.org>

# Conflicts:
#	contrib/devtools/lint-includes.sh
#	src/validation.cpp
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 27, 2020
@PastaPastaPasta
Merge bitcoin#13726: Utils and libraries: Removes the boost/algorithm…
fea0036 
…/string/join dependency

5f019d5 Removes the boost/algorithm/string/join dependency (251)

Pull request description:

  This commit removes the `boost/algorithm/string/join` dependency from the project by replacing `boost::algorithm::join` with the helper function proposed by @MarcoFalke in bitcoin#13726 (comment)

Tree-SHA512: d4ba3e7621b76bd5210aec9b8d6c320f7ee963d7f902e6d2d3fc0eadbee1cd77799e5c09be9c11452d2825f25740fc436cdec3a6b6c66ced674d771e4ed306ae
Signed-off-by: pasta <pasta@dashboost.org>

# Conflicts:
#	contrib/devtools/lint-includes.sh
#	src/validation.cpp
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Jul 27, 2020
@UdjinM6
Merge bitcoin#13726: Utils and libraries: Removes the boost/algorithm…
046be9c 
…/string/join dependency

5f019d5 Removes the boost/algorithm/string/join dependency (251)

Pull request description:

  This commit removes the `boost/algorithm/string/join` dependency from the project by replacing `boost::algorithm::join` with the helper function proposed by @MarcoFalke in bitcoin#13726 (comment)

Tree-SHA512: d4ba3e7621b76bd5210aec9b8d6c320f7ee963d7f902e6d2d3fc0eadbee1cd77799e5c09be9c11452d2825f25740fc436cdec3a6b6c66ced674d771e4ed306ae
Signed-off-by: pasta <pasta@dashboost.org>

# Conflicts:
#	contrib/devtools/lint-includes.sh
#	src/validation.cpp
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Jul 27, 2020
@UdjinM6
Merge bitcoin#13726: Utils and libraries: Removes the boost/algorithm…
d9ff5b0 
…/string/join dependency

5f019d5 Removes the boost/algorithm/string/join dependency (251)

Pull request description:

  This commit removes the `boost/algorithm/string/join` dependency from the project by replacing `boost::algorithm::join` with the helper function proposed by @MarcoFalke in bitcoin#13726 (comment)

Tree-SHA512: d4ba3e7621b76bd5210aec9b8d6c320f7ee963d7f902e6d2d3fc0eadbee1cd77799e5c09be9c11452d2825f25740fc436cdec3a6b6c66ced674d771e4ed306ae
Signed-off-by: pasta <pasta@dashboost.org>

# Conflicts:
#	contrib/devtools/lint-includes.sh
#	src/validation.cpp
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
2 more reviewers

@Empact Empact Empact left review comments

@maflcko maflcko maflcko left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@l2a5b1 @Empact @practicalswift @sipa @maflcko @fanquake