-
Notifications
You must be signed in to change notification settings - Fork 38.8k
doc: CChainState return values #16757
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
Conversation
fa8ec77 to
fa0b359
Compare
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, 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. |
fa0b359 to
fa79e17
Compare
fa79e17 to
fa99efd
Compare
| * | ||
| * @returns true unless a system error occurred | ||
| */ | ||
| bool CChainState::ActivateBestChainStep(CValidationState& state, const CChainParams& chainparams, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the method is defined there, shouldn't this documentation be in the .h file? (same for CChainState::ActivateBestChain below)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ActivateBestChainStep is marked private and was never meant to be exposed in the header. I think it makes sense to keep the documentation close to the function implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point for the public method, though. So I fixed this for ABC only
|
Added a commit that can be reviewed with the git diff options |
|
ACK fa912a8 |
fa912a8 doc: move-only ActivateBestChain doxygen comment to header (MarcoFalke) fa99efd doc: ActivateBestChainStep return value (MarcoFalke) Pull request description: It will always return true, unless a system error such as #15305 occurred ACKs for top commit: laanwj: ACK fa912a8 Tree-SHA512: d439da844a467f9705014b946d7d987fb62cb63fe6a325b2fdbbb73a6578fc0ade3f60892044f02face43948204fc4e3c9fa70d108233d4ca8eef27984059689
fa912a8 doc: move-only ActivateBestChain doxygen comment to header (MarcoFalke) fa99efd doc: ActivateBestChainStep return value (MarcoFalke) Pull request description: It will always return true, unless a system error such as bitcoin#15305 occurred ACKs for top commit: laanwj: ACK fa912a8 Tree-SHA512: d439da844a467f9705014b946d7d987fb62cb63fe6a325b2fdbbb73a6578fc0ade3f60892044f02face43948204fc4e3c9fa70d108233d4ca8eef27984059689
Summary:
fa912a8ad5a94cd2bdc149400b1befb346621f03 doc: move-only ActivateBestChain doxygen comment to header (MarcoFalke)
fa99efd054c57cd6717391f9ae8ce32b06986ff8 doc: ActivateBestChainStep return value (MarcoFalke)
Pull request description:
It will always return true, unless a system error such as #15305 occurred
ACKs for top commit:
laanwj:
ACK fa912a8ad5a94cd2bdc149400b1befb346621f03
---
Backport of Core [[bitcoin/bitcoin#16757 | PR16757]]
Test Plan:
read the comments
Reviewers: #bitcoin_abc, deadalnix
Reviewed By: #bitcoin_abc, deadalnix
Differential Revision: https://reviews.bitcoinabc.org/D7958
It will always return true, unless a system error such as #15305 occurred