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

> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>>> 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
> 
> I am of two minds.
> 
> I do not know if this is better for longer-term maintainability or
> not.  Such a message at the beginning of the function declares that
> the current implementation does not want to receive a command that
> is not either of these two.  Such a message tells the next developer
> who adds another caller to the function to make sure not to pass
> other commands, which is good.
> 
> But I do not know what it tells to the next developer who wants to
> add support for another command.  After opening that initial gate a
> bit wider, perhaps to
> 
> 	if (!is_filxup(command) && !is_my_new_command(command))
> 		BUT("not a FIXUP or SQUASH or MYCOMMAND %d", command);
> 
> would they make sure to handle their new command correctly in the
> if/elseif cascade, from which we are removing the "error()" with
> such a change, and would our reviewers notice if they forget to do
> so?  I dunno.