Thread (19 messages) 19 messages, 3 authors, 2025-03-19

Re: [PATCH v2 2/3] ci: add build checking for side-effects in assert() calls

From: Taylor Blau <hidden>
Date: 2025-03-19 22:26:21

On Wed, Mar 19, 2025 at 09:21:59AM -0700, Elijah Newren wrote:
quoted
I wonder if it might be useful to explain this in
Documentation/CodingGuidelines as a follow-up to this series. I was
thinking of a scenario where someone either writes a side-effecting
assert(), or a non-side-effecting one that is too complicated to prove
otherwise.

If that person runs 'make test' locally, they might not see any
failures, but then be surprised when CI fails on the new step. It may be
worth mentioning that we have such a check, and that we expect all
assert() statements to be side effect-free, and that developers can
verify this by ci/check-unsafe-assertions.sh.
The same could be said for coccinelle patches, hdr-check, check-pot,
fuzz tests, asan/ubsan, GIT_TEST_SPLIT_INDEX, pedantic build, osx, vs.
windows vs. linux, and perhaps others, which users won't catch on
'make test' locally but can result in failed CI builds and aren't
mentioned in CodingGuidelines.  I usually think of CodingGuidelines as
being the place for documenting things that can't be tested in an
automated fashion, and a brief mention that both cross platform and
additional more thorough but non-default tests can go in
SubmittingPatches.
Fair enough ;-).

Thanks,
Taylor
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help