ci: add -Wextra-semi to default config #332

pull fanquake wants to merge 1 commits into bitcoin-core:master from fanquake:drop_redundant_semi changing 1 files +1 −1

fanquake commented at 10:17 AM on August 4, 2026: member

Follow-up to https://github.com/bitcoin-core/libmultiprocess/commit/bd508311b5617bd8d31b875d8781392f2a6ad7be.
DrahtBot commented at 10:17 AM on August 4, 2026: none



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



Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

ACK hebasto, ryanofsky

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

No conflicts as of last run.
in test/mp/test/test.cpp:577 in bf3dbbc063
```
 574 |                      done.set_value();
 575 |                  }));
 576 |      });
 577 |      done.get_future().get();
 578 | -    KJ_EXPECT(error_thrown);
 579 | +    KJ_EXPECT(error_thrown)
```
maflcko commented at 10:34 AM on August 4, 2026:

hmm, for macros, I generally prefer if they enforce a trailing semicolon.

Though, it may be too late to fix this now?

hebasto commented at 10:50 AM on August 4, 2026:

Then perhaps omit -Wextra-semi-stmt?
maflcko commented at 10:35 AM on August 4, 2026: contributor

Seems fine, but I am -0
fanquake commented at 10:44 AM on August 4, 2026: member

Yea. I don't have too much of an opinion, but it wasn't clear to me why a PR labelled "CI" (#321) was also making random source code changes (bd508311b5617bd8d31b875d8781392f2a6ad7be) (although it seems like that PR was generally confusing (https://github.com/bitcoin-core/libmultiprocess/pull/321#issuecomment-5171132506)), so thought we could at least make this consistent.
maflcko commented at 10:51 AM on August 4, 2026: contributor

I think 321 was dropping it after a function def, this one is dropping it after statements.
hebasto commented at 10:51 AM on August 4, 2026: member

... random source code changes (bd50831)...

See #321 (review).
fanquake commented at 10:57 AM on August 4, 2026: member

See #321 (review).

Thanks. It would be good to write commit messages that contain information for other contributors (which then persists over time), so they don't have to search through GitHub comments to try and find context/explanations.
ci: add -Wextra-semi to default config 49834b2609
fanquake force-pushed on Aug 6, 2026
fanquake renamed this:
~~refactor: drop redundant semicolons~~
ci: add -Wextra-semi to default config
on Aug 6, 2026
fanquake commented at 11:18 AM on August 6, 2026: member

Changed to just add -Wextra-semi, to prevent some re-introduction.
hebasto approved
hebasto commented at 11:49 AM on August 6, 2026: member

ACK 49834b2609e3e4dca1b9cba36ee152552e221039.
in ci/configs/default.bash:3 in 49834b2609
```
   0 | @@ -1,5 +1,5 @@
   1 |  CI_DESC="CI job using default libraries and tools, and running IWYU"
   2 |  CI_DIR=build-default
   3 | -export CXXFLAGS="-Werror -Wall -Wextra -Wpedantic -Wno-unused-parameter"
   4 | +export CXXFLAGS="-Werror -Wall -Wextra -Wpedantic -Wno-unused-parameter -Wextra-semi"
```
ryanofsky commented at 8:21 PM on August 11, 2026:

In commit "ci: add -Wextra-semi to default config" (49834b2609e3e4dca1b9cba36ee152552e221039)

This could be good to add to llvm.bash as well in case gcc/clang behaviors are different

fanquake commented at 10:15 AM on August 12, 2026:

Done in #341.
ryanofsky approved
ryanofsky commented at 8:27 PM on August 11, 2026: collaborator

Code review ACK 49834b2609e3e4dca1b9cba36ee152552e221039. Tested locally and confirmed this catches both extra semicolons with bd508311b5617bd8d31b875d8781392f2a6ad7be reverted
ryanofsky merged this on Aug 11, 2026
ryanofsky closed this on Aug 11, 2026
fanquake deleted the branch on Aug 12, 2026

Type	Reviewers
ACK	hebasto, ryanofsky

Contributors

Linked (view graph)

#341 ci: add -Wextra-semi to llvm config

ci: add -Wextra-semi to default config #332

Reviews

Conflicts