Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Dec 8, 2021

Incorrect named args are source of bugs, like #22979.

To allow them being checked by clang-tidy, use a format it can understand.

-BEGIN VERIFY SCRIPT-
 sed -i -e 's|, /\* optional \*/ true,|, /*optional=*/true,|g' $( git grep -l ', /\* optional \*/ true,' )
-END VERIFY SCRIPT-
@maflcko
Copy link
Member Author

maflcko commented Dec 8, 2021

PSA: Also, please do use /*optional=*/true where needed. It wasted a lot of my time creating a follow-up after literally every merged RPC change to add the missing optional.

Alternatively, please review #23083

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 8, 2021

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23718 (PSBT: hash preimages fields by darosior)
  • #23508 (Add getdeploymentinfo RPC by ajtowns)
  • #23330 (WIP: [RPC] allow fetching headers by pages by JeremyRubin)
  • #22558 (psbt: Taproot fields for PSBT by achow101)
  • #22016 (rpc: add period_start to version bits statistics by Sjors)
  • #21283 (Implement BIP 370 PSBTv2 by achow101)
  • #19888 (rpc, test: Improve getblockstats for unspendables by fjahr)

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

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK fa9aaf8 - checked clang-tidy and it's fine here, (but throwing errors in other files. i.e wallet/test/wallet_tests.cpp).

@fanquake fanquake merged commit 7908772 into bitcoin:master Dec 9, 2021
@maflcko maflcko deleted the 2112-docOptName branch December 9, 2021 12:46
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 10, 2021
RandyMcMillan pushed a commit to RandyMcMillan/mempool-tab that referenced this pull request Dec 23, 2021
317dbc4 scripted-diff: Use named args in RPC docs (MarcoFalke)

Pull request description:

  Incorrect named args are source of bugs, like #22979.

  To allow them being checked by `clang-tidy`, use a format it can understand.

ACKs for top commit:
  fanquake:
    ACK 317dbc4 - checked `clang-tidy` and it's fine here, (but throwing errors in other files. i.e `wallet/test/wallet_tests.cpp`).

Tree-SHA512: e09dae8ee999a5c4819e6f848c12139593ca0e915e645c8fabeb97c379188fb9104d286c02c71f590abc64cdec125f78026735f83e016111976baa49d588a9bc
@bitcoin bitcoin locked and limited conversation to collaborators Dec 9, 2022
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.

3 participants