Subject: Re: [PATCH v4 03/15] bugreport: gather git version and build info

From: Emily Shaffer <[email-masked]>
Date: Thu, 19 Dec 2019 17:25:24 -0800

On Tue, Dec 17, 2019 at 12:34:53PM -0800, Junio C Hamano wrote:
> Johannes Schindelin <[email-masked]> writes:
> 
> >> +	if (build_options) {
> >> +		strbuf_addf(buf, "cpu: %s\n", GIT_HOST_CPU);
> >> +		if (git_built_from_commit_string[0])
> >> +			strbuf_addf(buf, "built from commit: %s\n",
> >> +			       git_built_from_commit_string);
> >> +		else
> >> +			strbuf_addf(buf, "no commit associated with this build\n");
> >
> > The "StaticAnalysis" job of the Azure Pipeline is not happy with this,
> > claiming that this should be an `strbuf_addstr()` call instead.
> 
> You mean the "else" clause, right?  That feels similar to say
> 
> 	printf("Hello world\n");
> 
> should better be written as
> 
> 	fputs("Hello world\n", stdout);
> 
> which I do not agree with at all.  IOW, I view the distinction more
> like "once it is written one way or the other way, it is not worth
> spending bits and braincycles to see if it is worth changing it"
> kind of minor stylistic preference.

I think I side with Junio here, although it's true that when
strbuf_addstr() exists it doesn't make that much sense to use
strbuf_addf(). Since there's some other comments, though, I'll change
this too to make your CI shut up. :)

 - Emily