Remove unnamed block in SendMessages function #14238

pull kostyantyn wants to merge 1 commits into bitcoin:master from kostyantyn:remove_unnamed_block_from_send_messages changing 1 files +465 −465

kostyantyn commented at 12:29 PM on September 17, 2018: contributor

This unnamed root block doesn't give any benefits as it wraps the entire function body but having it makes it harder to read this long function because of an extra indentation.

Remove unnamed block in SendMessages function

This unnamed root block doesn't give any benefits as it wraps the entire
function body but having it makes it harder to read this long function
because of extra indentation.

c9ee9a1ae3

kostyantyn commented at 12:30 PM on September 17, 2018: contributor

For reviewers: it's easier to see the actual change by adding &w=1 to the PR URL.
fanquake added the label Refactoring on Sep 17, 2018
fanquake added the label P2P on Sep 17, 2018
DrahtBot commented at 12:35 PM on September 17, 2018: member
Note to reviewers: This pull request conflicts with the following ones:
- #14220 (Transaction relay improvements by sdaftuar)
- #13548 (Document assumptions made in PeerLogicValidation::SendMessages(...) and rescanblockchain(...) by practicalswift)
- #13098 (Skip tx-rehashing on historic blocks by MarcoFalke)
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.
MarcoFalke commented at 8:09 PM on September 20, 2018: member
Thanks, but this can be change the next time someone touches the function. No need for a separate patch.

Pull requests without a rationale and clear improvement may be closed immediately.

Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly.
- Any test improvements or new tests that improve coverage are always welcome.
- Features are always welcome, but might be rejected early in the review process.
- Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed.
- Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they significantly improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the developer notes, stylistic code changes are usually rejected.
- All changes should have accompanying unit tests (see src/test/) or functional tests (see test/). PR authors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change.
Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time.
MarcoFalke closed this on Sep 20, 2018
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels