Re: [PATCH 1/5] notes-utils: handle boolean notes.rewritemode correctly

From: David Kastrup <dak@gnu.org>
Date: Feb 18, 2014, 09:36 UTC
Message-ID: <87mwhosqyg.fsf@fencepost.gnu.org>
In-Reply-To: <20140218090156.GC2692@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 10 quoted lines

> On Tue, Feb 18, 2014 at 09:41:51AM +0100, David Kastrup wrote:
>
>> gcc's flow analysis works with the same data as humans reading the
>> code.  If there is no information content in the function call, it makes
>> more sense to either making it void.
>
> The point of error() returning a constant -1 is to use this idiom:
>
>   if (something_failed)
>           return error("this will get printed, and we get a -1 return");

    if (something_failed)
        return error("this will get printed"), -1;

Not much of an idiom, no insider logic hidden to both compiler and reader.

Show 9 quoted lines

>>From a code perspective it's pointless. You could "just" write:
>
>   if (something_failed) {
>           error(...);
>           return -1;
>   }
>
> which is equivalent. But the point is that the former is shorter and a
> little more readable, assuming you are familiar with the idiom.

Assuming that.

Show 6 quoted lines

>> One can always explicitly write
>> 
>>   (config_error_nonbool("panic-when-assailed"), -1)
>
> Yes, but again, the point is readability. Doing that at each callsite is
> ugly and annoying.

I consider insider semantics ugly and annoying.  To each his own.

> You are the first person to ask about it, so there is no stock answer.
> However, everything I told you was in the commit messages and the list
> archive already. We can also avoid questions being asked by using
> those tools.

It's further raising the barriers for contributors if they are expected to have studied the last few years in the mailing archive. And the skills needed for that kind of study are mostly unrelated to good programming skills.

So I am less than convinced that this is among the coding practices that can be expected to attract/scare away potential contributors in proportion to their expected capability of advancing/hindering the project.

It's not me running the shop, so it's nothing more than an opinion.

-- 
David Kastrup