From: Lorenzo Pegorari <lorenzo.pegorari2002@gmail.com>
Date: Wed, 25 Mar 2026 21:33:00 GMT
Subject: Re: [GSoC PATCH v2 1/4] pack-write: add explanation to promisor file content
Message-ID: <acRUjAG7QOD5kVMI@lorenzo-VM>
In-Reply-To: <xmqqmrzy45m4.fsf@gitster.g>

On Mon, Mar 23, 2026 at 02:07:31PM -0700, Junio C Hamano wrote:
> LorenzoPegorari <lorenzo.pegorari2002@gmail.com> writes:
> 
> > In the entire codebase there is no explanation as to why the ".promisor"
> > files may contain the ref names (and their associated hashes) that were
> > fetched at the time the corresponding packfile was downloaded.
> >
> > Add comment explaining that these pieces of information are used only for
> > debugging reasons, and how they can be used while debugging.
> >
> > Signed-off-by: LorenzoPegorari <lorenzo.pegorari2002@gmail.com>
> 
> A natural question any reader of the above (and below) would be
> asking is: Who told you that these are only to aid debugging?
> 
> Please refer to the commit that brought in the reasoning behind the
> comment to make it more convincing.  
> 
> Something like this replacing the second paragraph,
> 
>     As explained in the log message of the commit 5374a290
>     (fetch-pack: write fetched refs to .promisor, 2019-10-14), where
>     this loop originally came from, these ref values are not
>     actually used for anything in the production, but are solely
>     there to help debugging.  Explain it in a new comment.
> 
> perhaps?

Makes perfect sense. I should have done this from the start. Thanks for
pointing that out.

> > +	/*
> > +	* Write in the .promisor file the ref names and associated hashes,
> > +	* obtained by fetch-pack, at the point of generation of the
> > +	* corresponding packfile. These pieces of info are only used to make
> > +	* it easier to debug issues with partial clones, as we can identify
> > +	* what refs (and their associated hashes) were fetched at the time
> > +	* the packfile was downloaded, and if necessary, compare those hashes
> > +	* against what the promisor remote reports now.
> > +	*/
> 
> I do not want to sound too pedantic, but we align '*' asterisks in
> our multi-line comments, assuming tabwidth=8 and monospace:
> 
> 	/*
> 	 * Write in the .promisor ...
> 	...
> 	 * against what the promisor remote reports now.
> 	 */
> 
> Your second and subsequent lines lack a single whitespace after the
> leading tab used for indent.

You are not too pedantic! Ack.

> >  	for (i = 0; i < nr_sought; i++)
> >  		fprintf(output, "%s %s\n", oid_to_hex(&sought[i]->old_oid),
> >  			sought[i]->name);