Re: [PATCH 2/3] graph: replace assert() with graph_assert() macro

On 1/7/2020 10:51 AM, Eric Sunshine wrote:

On Tue, Jan 7, 2020 at 10:36 AM Jeff King [off-list ref] wrote:

quoted

On Tue, Jan 07, 2020 at 02:55:46PM +0000, Derrick Stolee via GitGitGadget wrote:

quoted

The assert() macro is sometimes compiled out. Instead, switch these into
BUG() statements using our own custom macro.

I can buy the argument that compiling with and without NDEBUG can lead
to confusion. But if that is the case, wouldn't it be so for all of the
assert() calls, not just ones in the graph code?

This wasn't just a matter of potential confusion. It's one thing to
have assert()s in the code in general, but another thing when a
scripted test specifically depends upon the asserted condition, as was
the case with the test as originally proposed. Since the final patch
series removes that particular assert() altogether, it's perhaps not
that important anymore.

I'm happy to drop this commit, too. I misunderstood your point.

-Stolee