Hi,
On Wed, 10 Nov 2021, Ævar Arnfjörð Bjarmason wrote:
>
> On Tue, Nov 09 2021, Anders Kaseorg wrote:
>
> > + if (!worktree || !worktree->path)
> > + BUG("worktree->path must be non-NULL");
>
> Perhaps a metter of taste, but I think BUG() should really be used for
> things that need a custom message over and beyond what assert() gives
> us.
>
> In this case using BUG() gives you a worse message, if you do:
>
> assert(worktree && worktree->path)
>
> You'll get a sensible message from any modern compiler quotign the
> variable etc, all of which says the same thing as that BUG() message,
> just with less verbosity.
Maybe code reviews should stay away from contentious matters of taste.
This claim that `assert()` would somehow be preferable to `BUG()` is not
backed up by our very own coding guidelines. See for yourself:
https://github.com/git/git/blob/v2.33.1/Documentation/CodingGuidelines
does not mention it.
The question of `assert()` vs `BUG()` has been brought up on this mailing
list before, without a clear preference for `assert()`, in contrast to
what the comment quoted above would want to make believe.
And the fact that BUG() allows for a well-crafted message without having
to rely on the compiler to guess as to what would make for a helpful
message, that alone speaks volumes.
Ciao,
Johannes
