8000 refactor: Move stopafterblockimport option out of blockstorage by TheCharlatan · Pull Request #28053 · bitcoin/bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

refactor: Move stopafterblockimport option out of blockstorage #28053

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

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

TheCharlatan
Copy link
Contributor
@TheCharlatan TheCharlatan commented Jul 8, 2023

This has the benefit of moving this StartShutdown call out of the blockstorage file and thus out of the kernel's responsibility. The user can now decide if he wants to start shutdown / interrupt after a block import or not.

This also simplifies #28048, making it one fewer shutdown call to handle.

@DrahtBot
Copy link
Contributor
DrahtBot commented Jul 8, 2023

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

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK MarcoFalke, ryanofsky
Stale ACK furszy

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #28051 (Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly by ryanofsky)
  • #28048 (kernel: Remove StartShutdown calls from validation code by ryanofsky)
  • #27607 (index: make startup more efficient by furszy)

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.

Copy link
Member
@furszy furszy left a comment

Choose a reason for hiding this comment

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

ACK 5867bb1

Copy link
Contributor
@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Code review ACK 5867bb1. This looks good, and now can be rebased and simplified because #27607 is merged

@TheCharlatan
Copy link
Contributor Author

Rebased 5867bb1 -> f87c21c (blockImportReturn_0 -> blockImportReturn_1, compare)

This has the benefit of moving the StartShutdown call out of the
blockstorage file and thus out of the kernel's responsibility. The user
can now decide if he wants to start shutdown / interrupt after a block
import or not.
@TheCharlatan
Copy link
Contributor Author

Updated f87c21c -> 462390c (blockImportReturn_1 -> blockImportReturn_2, compare)

Copy link
Member 8000
@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

lgtm ACK 462390c 🗝

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: lgtm ACK 462390c85f1174b3cf83dfb0f74922049e5cb8af  🗝
79pOWJJhRKkMSczcP5SRwipfYj5m6V7y44rmI0Q0c7UbllpH+RG/Ei49ay9gaV/dBEYrjhKR0IAUOAi4s+THDw==

@DrahtBot DrahtBot requested review from furszy and ryanofsky July 11, 2023 10:11
Copy link
Contributor
@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

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

Code review ACK 462390c. Just has been rebased and is a simpler change after #27607

@ryanofsky ryanofsky merged commit e253568 into bitcoin:master Jul 11, 2023
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jul 12, 2023
@bitcoin bitcoin locked and limited conversation to collaborators Jul 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Done or Closed or Rethinking
Development

Successfully merging this pull request may close these issues.

5 participants
0