Re: [PATCH 3/3] fixup! reftable: add a heap-based priority queue for reftable records

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>> I agree that the patch as posted does not help but if this is
>> originally an assertion, then it should never trigger in real life,
>> so BUG() would be more appropriate than an error return, no?
>
> My thinking was that it doesn't make much sense as an assertion in the
> first place. It is not a side effect of "let's make sure things are as
> we expect while we're doing some other operation". The whole point of
> the function is: is this data structure properly in order.

Very true.  Ah, so you mean the way this function is supposed to be
used is to _call_ it, like so:

	if (!is_our_data_structure_healthy())
		BUG(...);

It makes it easier to reason about what the function is doing, I
guess.

> But I guess you could argue that calling the function is itself a form
> of assertion. I don't really care that much either way, so whatever
> Han-Wen prefers is fine with me (but I do think it is worth addressing
> the warning Carlo found _somehow_).
>
> -Peff