Re: [RFC PATCH 1/9] rebase -i: only write fixup-message when it's needed
- From
Charvi Mendiratta <charvi077@gmail.com>
- Date
- Jan 16, 2021, 04:49 UTC
- Message-ID
- <CAPSFM5cq6kVM5tUWWKryFvmQBbNzOaOf-SR-Q4wRvs-4Cvc-fA@mail.gmail.com>
- In-Reply-To
- <xmqqbldqt8rm.fsf@gitster.c.googlers.com>
Hi Junio,
On Fri, 15 Jan 2021 at 22:52, Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
>
> Charvi Mendiratta <charvi077@gmail.com> writes:
>
> > Okay, I looked into the write_message(...) and agree that it does not return
> > a positive value and only returns non-zero for error case and zero for
> > success. So, for this patch maybe we can ignore checking '< 0' here and
> > later add another patch to make this function follow the convention of
> > "negative is error".
>
> Please don't. There is a higher cognitive cost to readers when you write
>
> if (write_message(...)) {
>
> The reader is forced to look at its implementation to see if it
> returns positive in a non-error situation.
>
> If you write it like so from the beginning
>
> if (write_message(...) < 0) {
>
> the reader can trust that the code follows "negative is an error"
> convention. One fewer thing readers have to worry about.I agree, earlier I was confused as there were many instances of write_message() without '< 0' in sequencer.c but considering the above I completely agree to follow the convention.
Thanks and Regards, Charvi