Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Nov 16, 2021

Argument names of nInIn are not helpful.

Argument names of "nInIn" are not helpful.
@katesalazar
Copy link
Contributor

Concept ACK.

Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

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

ACK fa54a40

The variable names are now more understandable and follow snake_case naming convention, rather than camel case convention, which is in line with bitcoin’s naming convention.
Similarly, direct initialization (()) is replaced with direct list initialization ({}) for initializing member variables of the MutableTransactionSignatureCreator class.

I have checked for clang-formatting for the changes introduced, and I found no formatting issues either.

@fanquake fanquake requested a review from achow101 November 17, 2021 00:39
@DrahtBot
Copy link
Contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #22793 (Simplify BaseSignatureChecker virtual functions and GenericTransactionSignatureChecker constructors by achow101)

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

@achow101 achow101 left a comment

Choose a reason for hiding this comment

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

ACK fa54a40

@maflcko maflcko merged commit 398fd63 into bitcoin:master Nov 17, 2021
@maflcko maflcko deleted the 2111-docMutableTransactionSignatureCreator branch November 17, 2021 07:19
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 17, 2021
…onSignatureCreator

fa54a40 doc: Pick better named args for MutableTransactionSignatureCreator (MarcoFalke)

Pull request description:

  Argument names of `nInIn` are not helpful.

ACKs for top commit:
  shaavan:
    ACK fa54a40
  achow101:
    ACK fa54a40

Tree-SHA512: 53a38588fdee07d7896a66339c1c2c2355638db95a95cad9844b60cd34e935bb726ab64d0c42dc414beb35375e56440f8a9cb3fbf5aec55c1eed066b7acad8c8
@bitcoin bitcoin locked and limited conversation to collaborators Nov 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants