Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Aug 29, 2019

It will always return true, unless a system error such as #15305 occurred

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 29, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #16743 (refactor: move LoadChainTip/RelayBlocks under CChainState by jamesob)
  • #16324 (Get cs_main out of the critical path in ProcessMessages by TheBlueMatt)
  • #16323 (Call ProcessNewBlock() asynchronously by TheBlueMatt)

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.

@maflcko maflcko changed the title doc: ActivateBestChainStep return value doc: CChainState return values Aug 29, 2019
*
* @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)
Copy link
Member

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)

Copy link
Member Author

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

Copy link
Member Author

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

@maflcko
Copy link
Member Author

maflcko commented Sep 10, 2019

Added a commit that can be reviewed with the git diff options --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space.

@maflcko maflcko added this to the 0.19.0 milestone Sep 10, 2019
@laanwj
Copy link
Member

laanwj commented Sep 16, 2019

ACK fa912a8

laanwj added a commit that referenced this pull request Sep 16, 2019
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
@laanwj laanwj merged commit fa912a8 into bitcoin:master Sep 16, 2019
@maflcko maflcko deleted the 1909-docValABCS branch September 16, 2019 13:07
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 16, 2019
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
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 16, 2020
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
kwvg added a commit to kwvg/dash that referenced this pull request Oct 22, 2021
pravblockc pushed a commit to pravblockc/dash that referenced this pull request Nov 18, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants