Skip to content

Followups for getdeploymentinfo #24187

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Feb 14, 2022
Merged

Followups for getdeploymentinfo #24187

merged 5 commits into from
Feb 14, 2022

Conversation

ajtowns
Copy link
Contributor

@ajtowns ajtowns commented Jan 28, 2022

Documentation, comments and trivial code changes to followup #23508.

@ajtowns ajtowns mentioned this pull request Jan 28, 2022
Merged
maflcko
maflcko reviewed Jan 28, 2022
View reviewed changes
@maflcko
Copy link
Member

maflcko commented Jan 28, 2022

Maybe also #23508 (comment) ?

@ajtowns ajtowns added this to the 23.0 milestone Jan 28, 2022
@ajtowns ajtowns force-pushed the 202201-getdepinfo-extra branch from 3057227 to 793f6af Compare January 28, 2022 08:14
@ajtowns
Copy link
Contributor Author

ajtowns commented Jan 28, 2022

Was leaving the SoftForkPushBackDesc argument name change 'til other work that would also touch that, but meh, added.

maflcko
maflcko approved these changes Jan 28, 2022
View reviewed changes
Copy link
Member

@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

@luke-jr
Copy link
Member

luke-jr commented Jan 31, 2022

We haven't used the variable style behind pindex for years now. Why rename to it?

@Sjors
Copy link
Member

Sjors commented Jan 31, 2022 •
edited
Loading

We haven't used the variable style behind pindex for years now. Why rename to it?

We can name it something else (as long as it's clear we're not necessarily dealing with the tip). block_index or just index (I used the latter in the getblockfrompeer RPC code)?

@ajtowns ajtowns force-pushed the 202201-getdepinfo-extra branch from 793f6af to e5f0356 Compare February 4, 2022 03:44
@ajtowns
Copy link
Contributor Author

ajtowns commented Feb 4, 2022

Changed to blockindex.

@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 4, 2022 •
edited
Loading

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24118 (Add 'sweepwallet' RPC by Xekyo)
  • #21702 (Implement BIP-119 Validation (CheckTemplateVerify) by JeremyRubin)

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.

@JeremyRubin
Copy link
Contributor

shrug about renaming tip, easy enough to fix the conflict either way.

@Sjors
Copy link
Member

Sjors commented Feb 4, 2022 •
edited
Loading

utACK e5f0356

@JeremyRubin a rebase beats an off by one soft fork activation error due to unreadable activation-related code :-)

@DrahtBot DrahtBot mentioned this pull request Feb 4, 2022
Closed
@DrahtBot DrahtBot mentioned this pull request Feb 11, 2022
Merged
@maflcko maflcko merged commit af0b578 into bitcoin:master Feb 14, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 14, 2022
Merge bitcoin#24187: Followups for getdeploymentinfo
2d95772 
e5f0356 rpc/blockchain: rename getdeploymentinfo tip/active_chain_tip to blockindex (Anthony Towns)
fbab43f rpc/blockchain: a constant craving (Anthony Towns)
5179656 trivial: comment tweaks (Anthony Towns)
32f04e6 rpc documentation improvements (Anthony Towns)
555eafa doc: getdeploymentinfo release notes tweaks (Anthony Towns)

Pull request description:

  Documentation, comments and trivial code changes to followup bitcoin#23508.

ACKs for top commit:
  Sjors:
    utACK e5f0356

Tree-SHA512: 4e854a8453588901edb887504f7bfa100cc32df2e99654a5e5970032a0bd63259ba0582479e15bc09ef4792c6672715007f89eb1a7b2d7e229433a678cde9f44
@bitcoin bitcoin locked and limited conversation to collaborators Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
1 more reviewer

@maflcko maflcko maflcko approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Refactoring
Projects
None yet
Milestone
23.0
Development

Successfully merging this pull request may close these issues.
6 participants
@ajtowns @maflcko @luke-jr @Sjors @DrahtBot @JeremyRubin