From: Tian Yuchen <a3205153416@gmail.com>
Date: Fri, 03 Apr 2026 17:40:55 GMT
Subject: Re: [PATCH v4 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms()
Message-ID: <5017740b-4437-4e55-b019-244b33eed05a@gmail.com>
In-Reply-To: <20260403120938.1142533-2-shreyanshpaliwalcmsmn@gmail.com>

On 4/3/26 20:08, Shreyansh Paliwal wrote:
> -long get_files_ref_lock_timeout_ms(void)
> +long get_files_ref_lock_timeout_ms(struct repository *repo)
>   {
>   	static int configured = 0;
>   
> @@ -997,7 +997,7 @@ long get_files_ref_lock_timeout_ms(void)
>   	static int timeout_ms = 100;
> 

A very minor and trivial question: the 'static' keyword is still present 
here. This is entirely understandable, given that you mentioned earlier 
that...

> Hi Yuchen,
> 
> I have acknowledged this in a previous reply to Burak. As stated there,
> this is a valid issue and would require moving the config into
> repo-settings struct.
> In this patch, I focused on removing the dependency on
> 'the_repository' while preserving existing behavior. Global state
> removal and multi-repo correctness is an incremental process,
> so I would prefer to handle this in a follow-up change.
> I'll also update the patch title in the next version to better reflect
> the scope of the change.

But if that is the case, the accuracy of this line in the commit message:

> This reduces reliance on the_repository global.

..is open to question. Or perhaps it would be worth mentioning:

"Note: This function still uses static variables, which means it does 
not fully support in-process multi-repo usage yet. This will be 
addressed in a follow-up by moving the configuration to the 
'repo-settings' struct, but changing the signature is a necessary first 
step..."

or something (shorter)?

To reiterate, I think this is a minor issue, so it would be better if 
you decide for yourself. Other parts look good to me. ;)

Regards, Yuchen