From: Eric Sunshine Date: Mon, 07 Oct 2024 03:26:24 GMT Subject: Re: [PATCH v2 1/4] worktree: refactor infer_backlink() to use *strbuf Message-ID: In-Reply-To: On Sun, Oct 6, 2024 at 10:42 PM Caleb White wrote: > On Sunday, October 6th, 2024 at 13:16, Eric Sunshine wrote: > > ... this code now becomes more than a little confusing to read. It > > says "if infer_backlink then signal error", which sounds rather > > backward and leaves the reader scratching his or her head. ("Why > > signal error if the function succeeded?"). Hence, infer_backlink() > > should probably return 1 on success and 0 on failure, which will make > > this code read more idiomatically: > > > > if (!infer_backlink(realdotgit.buf, &backlink)) { > > ...signal error... > > This was my first thought, however, on unix it is fairly standard > to return 0 if successful and a non-zero int if there's an error. > I don't mind updating, but I want to follow what makes the most > sense and would be most expected. I mentioned it because it made my reading hiccup, but I don't feel too strongly about it one way or the other considering that this is an internal function.