Simplify ProcessGetBlockData execution by removing send flag#13250
Closed
pstratem wants to merge 2 commits intobitcoin:masterfrom
Closed
Simplify ProcessGetBlockData execution by removing send flag#13250pstratem wants to merge 2 commits intobitcoin:masterfrom
pstratem wants to merge 2 commits intobitcoin:masterfrom
Conversation
Contributor
Author
|
the send flag effectively exits the function whenever it's set to false, this simply actually returns |
Member
|
Yes, I can read the code. But can you actually put that in the PR description? The description goes into the merge commit for example, and is helpful for people investigating the history of code. |
pstratem
force-pushed
the
2018-05-16-processgetblockdata
branch
from
1720603 to
311af6c
Compare
pstratem
changed the title
skeees
reviewed
May 16, 2018
src/net_processing.cpp
Outdated
Contributor
There was a problem hiding this comment.
you should return if pindex is not defined
pstratem
force-pushed
the
2018-05-16-processgetblockdata
branch
from
311af6c to
660e484
Compare
Setting the send flag to false can be replaced by simply returning.
pstratem
force-pushed
the
2018-05-16-processgetblockdata
branch
2 times, most recently
from
31214e8 to
b4b6cb4
Compare
Member
|
Travis: |
pstratem
force-pushed
the
2018-05-16-processgetblockdata
branch
from
b4b6cb4 to
261026b
Compare
Empact
reviewed
May 17, 2018
Empact
reviewed
May 17, 2018
Contributor
There was a problem hiding this comment.
Looks like you're using 2 nodes below, index 0 and 1, is that right?
pstratem
force-pushed
the
2018-05-16-processgetblockdata
branch
3 times, most recently
from
a1b4bb4 to
dd5ee7f
Compare
Contributor
| Needs rebase |
maflcko
reviewed
Jul 14, 2018
| # Setup the p2p connections and start up the network thread. | ||
| self.test_node = self.nodes[0].add_p2p_connection(TestP2PConn()) | ||
|
|
||
| network_thread_start() |
Member
There was a problem hiding this comment.
nit: The thread is always running on master. Remove this line.
Member
|
I've rebased and fixed the nit in #13670. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Setting the send flag to false can be replaced by simply returning.