Re: [PATCH v3 0/6] usage API: add and use a bug() + BUG_if_bug()

From: Glen Choo <chooglen@google.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>, git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Bagas Sanjaya" <bagasdotme@gmail.com>,
	"Abhradeep Chakraborty" <chakrabortyabhradeep79@gmail.com>,
	"Josh Steadmon" <steadmon@google.com>,
	"Andrei Rybak" <rybak.a.v@gmail.com>,
	"Emily Shaffer" <emilyshaffer@google.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [PATCH v3 0/6] usage API: add and use a bug() + BUG_if_bug()
Date: Thu, 02 Jun 2022 09:56:25 -0700	[thread overview]
Message-ID: <kl6lczfrdmx2.fsf@chooglen-macbookpro.roam.corp.google.com> (raw)
In-Reply-To: <cover-v3-0.6-00000000000-20220602T122106Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> This series adds a bug() (lower-case) function to go along with
> BUG(). As seen in 3-5/6 this makes it much easier to handle the cases
> such as parse-options.c where we'd like to call BUG(), but would like
> to first exhaustively accumulate the N issues we spot before doing so,
> and not merely BUG() out on the first one.
>
> Changes since v2[1]:
>
>  * Fix up commentary in 1/6 to address Junio's suggestions, and
>    de-duplicate previously added commentary (just the one existing
>    comment in git-compat-util.h sufficed).
>
>  * The BUG() function itself now clears bug_called_must_BUG, as
>    suggested by Glen. This simplifies things, and allows "BUG()" to be
>    called after a sequence of "BUG()" instead of "BUG_if_bug()". That
>    pattern is now used in 6/6.
>
>    I pondered adding a message that BUG() would emit in that case,
>    i.e. to say that we'd previously called bug() and were now calling
>    BUG(), but left it. I think the result in 6/6 is better as a
>    result.
>
>  * Used "else if" in 4/6 and got rid of "break" to exhaustively log
>    errors.
>
>  * Got rid of a "below the fold" part of a commit message.
>
> 1. https://lore.kernel.org/git/cover-v2-0.6-00000000000-20220531T164806Z-avarab@gmail.com/

I didn't look too closely at patch 1/6 (since I'm not too familiar with
the trace2 stuff), but everything else looks great :)

(not sure if this is enough to give a Reviewed-By)