Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Nov 18, 2021

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

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

@maflcko
Copy link
Member Author

maflcko commented Nov 18, 2021

Let's see how many conflicts this has 😅

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 18, 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:

  • #24165 (p2p: extend inbound eviction protection by network to CJDNS peers by jonatack)
  • #23604 (Use Sock in CNode by vasild)
  • #22910 (net: Encapsulate asmap in NetGroupManager by jnewbery)
  • #17167 (Allow whitelisting outgoing connections by luke-jr)

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 scripted-diff: Use clang-tidy syntax for C++ named arguments scripted-diff: Use clang-tidy syntax for C++ named arguments [WIP; TOO MANY CONFLICTS] Nov 18, 2021
@maflcko maflcko marked this pull request as draft November 18, 2021 20:03
@katesalazar
Copy link
Contributor

Concept ACK.

@maflcko
Copy link
Member Author

maflcko commented Jan 17, 2022

Running against c41736a:

Pretty sure those are bugs in current master and have nothing to do with this pull? Maybe create a separate issues or pull to fix those?

@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 4, 2022

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

MarcoFalke added 2 commits February 4, 2022 11:43
-BEGIN VERIFY SCRIPT-
 sed -i --regexp-extended -e 's_(\(|\{|, ?)\/\* ?([^=* ]+) ?\*\/ ?_\1/*\2=*/_g' $( git grep -l --extended-regexp ', ?\/\* ?[^=* ]+ ?\*\/' ./src )
 # perl -0777 -pi -e 's:((\(|\{|,)(\n| )*)\/\* ?([^=* ]+) ?\*\/ ?:\1/*\4=*/:g' $( git ls-files ./src/ )
-END VERIFY SCRIPT-
@maflcko
Copy link
Member Author

maflcko commented Feb 4, 2022

Closing as up for grabs

@fanquake
Copy link
Member

Closing as up for grabs

Picked up in #24661.

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Apr 4, 2022
…guments

37a16ff refactor: fix clang-tidy named args usage (fanquake)

Pull request description:

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

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

  Picks up bitcoin#23545, with some additional changes and some feedback addressed.

  With these changes invoking `./autogen.sh && ./configure CC=clang-12 CXX=clang++-12 && make clean && bear make -j9 && ( cd ./src/ && run-clang-tidy-12 -j9 )` no-longer results in named argument errors out of `clang-tidy`.

  Ultimately I think we should just add `clang-tidy-*` jobs to the CI and automate things away.

ACKs for top commit:
  MarcoFalke:
    cr ACK 37a16ff

Tree-SHA512: 9bfc0d006eb187755b4fdb0bd92cee9266fc0816be42065ef7dcd885b9020ff12e3cdd7ca3a831613a56a0206d448e690ee4e1fa37628fa2013860e17f416ff3
@bitcoin bitcoin locked and limited conversation to collaborators Mar 24, 2023
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.

6 participants