Re: [PATCH v2] sequencer: replace error() with BUG() in update_squash_messages()

From: Phillip Wood <phillip.wood123@gmail.com>
Date: May 29, 2025, 13:39 UTC
Message-ID: <edd4d392-f124-4e97-b4a9-6cf3fde1e804@gmail.com>
In-Reply-To: <xmqqr008peil.fsf@gitster.g>

On 28/05/2025 23:16, Junio C Hamano wrote:

Show 24 quoted lines

> "Lidong Yan via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> diff --git a/sequencer.c b/sequencer.c
>> index b5c4043757e..3cd0dd3434e 100644
>> --- a/sequencer.c
>> +++ b/sequencer.c
>> @@ -2139,7 +2139,7 @@ static int update_squash_messages(struct repository *r,
>>   		strbuf_add_commented_lines(&buf, body, strlen(body),
>>   					   comment_line_str);
>>   	} else
>> -		return error(_("unknown command: %d"), command);
>> +		BUG(_("unknown command: %d"), command);
>>   	repo_unuse_commit_buffer(r, commit, message);
> 
> BUG() is not end-user facing but programmer facing, and we do not
> use _("...") in them.  I see a few existing violators that need to
> be corrected.
> 
> OK.  Or
> 
> 	if (!is_fixup(command))
> 		BUG("not a FIXUP or SQUASH %d", command);
> 
> at the very beginning of the function?

Asserting the precondition at the start of the function sounds like a good idea

Best Wishes

Phillip