git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] pretend_sha1_file(): Change return type from int to void

From
Stefan Beller <sbeller@google.com>
Date
Oct 7, 2015, 21:24 UTC
Message-ID
<CAGZ79kYfcA4Atsx9zy+DNA_uhW-f91c5dLMGqhwSpEV7tPE5dA@mail.gmail.com>
In-Reply-To
<xmqqh9m2tm8v.fsf@gitster.mtv.corp.google.com>
On Wed, Oct 7, 2015 at 2:14 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
> Stefan Beller <sbeller@google.com> writes:
>
>> Compare to a patch[1] I sent a while back and the discussion on it.
>>
>> [1] https://www.mail-archive.com/git@vger.kernel.org/msg70474.html
>
> It is not clear what conclusion you want others to draw from the
> comparison, I am afraid.

I did not draw a conclusion. All I wanted is to point out, we've had similar patches before. Maybe I wanted to point out, you had a different opinion about the patch I linked to than you seem to have now about this patch.

Show 10 quoted lines
>
> I am guessing that you are in favor of dropping this patch, because
> 'int' that signals success or error is the most natural return type
> and meanint for this function if its callers ever start using the
> value as the indication of an error, just like in the old thread,
> the return value from get_remote_heads() had the most useful type
> and the meaning for its callers if they wanted to use it.
>
> And if that is what you wanted to say, I fully agree with the
> conclusion.

I really did not want to say anything except for pointing out how similar cases were dealt with in the past. So I guess for a good comparision we'd need to asses how similar the patches are. If they are similar it's easier to link to the old discussion instead of retyping the same reasons.

Show 6 quoted lines
>
> By the way, it is not a very good comparison, though.  The patch in
> the old thread deliberately attempted to discard a useful piece of
> information.  The information the patch in this thread attempts to
> discard is not so useful, as there currently is nobody that returns
> an error in the codepath.

Isn't that a bit picky? (old thread: the information is useful, but nobody uses it, this thread: information is useless, and nobody uses it)

So the similarity is nobody is using the result, the difference is the usefulness of the information provided.

>  So in that sense, the patch in this
> thread to change the return value to void is a bit more justifiable
> than the one in the old thread, I think.
That makes sense to me.
>
> Thanks.
Previous: Junio C HamanoNext: Junio C Hamano
Message 9 of 12 in “pretend_sha1_file(): Change return type from int to void”
  1. pretend_sha1_file(): Change return type from int to voidTobias Klauser, Oct 6, 2015
  2. Johannes SchindelinOct 6, 2015
  3. Tobias KlauserOct 6, 2015
  4. Johannes SchindelinOct 6, 2015
  5. Tobias KlauserOct 7, 2015
  6. Junio C HamanoOct 7, 2015
  7. Stefan BellerOct 7, 2015
  8. Junio C HamanoOct 7, 2015
  9. Stefan BellerOct 7, 2015
  10. Junio C HamanoOct 7, 2015
  11. Junio C HamanoOct 7, 2015
  12. Tobias KlauserOct 8, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.